Small load strlen opt follow-up improvement (PR tree-optimization/83444)

Message ID 20171219172355.GU2353@tucnak
State New
Headers show
Series
  • Small load strlen opt follow-up improvement (PR tree-optimization/83444)
Related show

Commit Message

Jakub Jelinek Dec. 19, 2017, 5:23 p.m.
Hi!

As the testcase shows, we weren't handling the case where we have
_34 = MEM_REF[&b, 4B];
- we would (if it exists) find the string length for b.a, but not
for the second field.  Using get_addr_stridx handles that well.
The reason to use get_stridx on the MEM_REF first operand is to handle
the case when the MEM_REF is based on SSA_NAME, that is something
get_addr_stridx doesn't handle.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-12-19  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/83444
	* tree-ssa-strlen.c (strlen_check_and_optimize_stmt): For the
	character load case, if get_stridx on MEM_REF's operand doesn't
	look usable, retry with get_addr_stridx.

	* gcc.dg/strlenopt-38.c: New test.


	Jakub

Comments

Richard Biener Dec. 19, 2017, 5:53 p.m. | #1
On December 19, 2017 6:23:55 PM GMT+01:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!

>

>As the testcase shows, we weren't handling the case where we have

>_34 = MEM_REF[&b, 4B];

>- we would (if it exists) find the string length for b.a, but not

>for the second field.  Using get_addr_stridx handles that well.

>The reason to use get_stridx on the MEM_REF first operand is to handle

>the case when the MEM_REF is based on SSA_NAME, that is something

>get_addr_stridx doesn't handle.

>

>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?


OK. 
Richard. 

>2017-12-19  Jakub Jelinek  <jakub@redhat.com>

>

>	PR tree-optimization/83444

>	* tree-ssa-strlen.c (strlen_check_and_optimize_stmt): For the

>	character load case, if get_stridx on MEM_REF's operand doesn't

>	look usable, retry with get_addr_stridx.

>

>	* gcc.dg/strlenopt-38.c: New test.

>

>--- gcc/tree-ssa-strlen.c.jj	2017-12-19 08:43:35.000000000 +0100

>+++ gcc/tree-ssa-strlen.c	2017-12-19 12:52:37.007267178 +0100

>@@ -3155,14 +3155,27 @@ strlen_check_and_optimize_stmt (gimple_s

> 	  {

> 	    tree off = integer_zero_node;

> 	    unsigned HOST_WIDE_INT coff = 0;

>-	    int idx = -1;

>+	    int idx = 0;

> 	    tree rhs1 = gimple_assign_rhs1 (stmt);

> 	    if (code == MEM_REF)

> 	      {

> 		idx = get_stridx (TREE_OPERAND (rhs1, 0));

>-		off = TREE_OPERAND (rhs1, 1);

>+		if (idx > 0)

>+		  {

>+		    strinfo *si = get_strinfo (idx);

>+		    if (si

>+			&& si->nonzero_chars

>+			&& TREE_CODE (si->nonzero_chars) == INTEGER_CST

>+			&& (wi::to_widest (si->nonzero_chars)

>+			    >= wi::to_widest (off)))

>+		      off = TREE_OPERAND (rhs1, 1);

>+		    else

>+		      /* This case is not useful.  See if get_addr_stridx

>+			 returns something usable.  */

>+		      idx = 0;

>+		  }

> 	      }

>-	    else

>+	    if (idx <= 0)

> 	      idx = get_addr_stridx (rhs1, NULL_TREE, &coff);

> 	    if (idx > 0)

> 	      {

>--- gcc/testsuite/gcc.dg/strlenopt-38.c.jj	2017-12-19

>08:43:35.000000000 +0100

>+++ gcc/testsuite/gcc.dg/strlenopt-38.c	2017-12-19 12:53:19.006730543

>+0100

>@@ -36,3 +36,14 @@ baz (void)

>   if (s.b[0] != 0)

>     abort ();

> }

>+

>+void

>+boo (void)

>+{

>+  struct S s;

>+  strcpy (s.b, "012");

>+  strcpy (s.c, "");

>+  strcpy (s.b, s.c);

>+  if (strlen (s.b) != 0)

>+    abort ();

>+}

>

>	Jakub
Jeff Law Dec. 19, 2017, 9:03 p.m. | #2
On 12/19/2017 10:23 AM, Jakub Jelinek wrote:
> Hi!

> 

> As the testcase shows, we weren't handling the case where we have

> _34 = MEM_REF[&b, 4B];

> - we would (if it exists) find the string length for b.a, but not

> for the second field.  Using get_addr_stridx handles that well.

> The reason to use get_stridx on the MEM_REF first operand is to handle

> the case when the MEM_REF is based on SSA_NAME, that is something

> get_addr_stridx doesn't handle.

> 

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

> 

> 2017-12-19  Jakub Jelinek  <jakub@redhat.com>

> 

> 	PR tree-optimization/83444

> 	* tree-ssa-strlen.c (strlen_check_and_optimize_stmt): For the

> 	character load case, if get_stridx on MEM_REF's operand doesn't

> 	look usable, retry with get_addr_stridx.

> 

> 	* gcc.dg/strlenopt-38.c: New test.

OK.
jeff

Patch

--- gcc/tree-ssa-strlen.c.jj	2017-12-19 08:43:35.000000000 +0100
+++ gcc/tree-ssa-strlen.c	2017-12-19 12:52:37.007267178 +0100
@@ -3155,14 +3155,27 @@  strlen_check_and_optimize_stmt (gimple_s
 	  {
 	    tree off = integer_zero_node;
 	    unsigned HOST_WIDE_INT coff = 0;
-	    int idx = -1;
+	    int idx = 0;
 	    tree rhs1 = gimple_assign_rhs1 (stmt);
 	    if (code == MEM_REF)
 	      {
 		idx = get_stridx (TREE_OPERAND (rhs1, 0));
-		off = TREE_OPERAND (rhs1, 1);
+		if (idx > 0)
+		  {
+		    strinfo *si = get_strinfo (idx);
+		    if (si
+			&& si->nonzero_chars
+			&& TREE_CODE (si->nonzero_chars) == INTEGER_CST
+			&& (wi::to_widest (si->nonzero_chars)
+			    >= wi::to_widest (off)))
+		      off = TREE_OPERAND (rhs1, 1);
+		    else
+		      /* This case is not useful.  See if get_addr_stridx
+			 returns something usable.  */
+		      idx = 0;
+		  }
 	      }
-	    else
+	    if (idx <= 0)
 	      idx = get_addr_stridx (rhs1, NULL_TREE, &coff);
 	    if (idx > 0)
 	      {
--- gcc/testsuite/gcc.dg/strlenopt-38.c.jj	2017-12-19 08:43:35.000000000 +0100
+++ gcc/testsuite/gcc.dg/strlenopt-38.c	2017-12-19 12:53:19.006730543 +0100
@@ -36,3 +36,14 @@  baz (void)
   if (s.b[0] != 0)
     abort ();
 }
+
+void
+boo (void)
+{
+  struct S s;
+  strcpy (s.b, "012");
+  strcpy (s.c, "");
+  strcpy (s.b, s.c);
+  if (strlen (s.b) != 0)
+    abort ();
+}