Patchwork Fix up reassoc range test optimization (PR tree-optimization/59388)

login
register
mail settings
Submitter Jakub Jelinek
Date Dec. 6, 2013, 8:33 p.m.
Message ID <20131206203307.GU892@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/298248/
State New
Headers show

Comments

Jakub Jelinek - Dec. 6, 2013, 8:33 p.m.
Hi!

range->exp can in some cases be equal to op, as in the following testcase:
  <unnamed-unsigned:1> _2;
  int _3;
  _Bool _4;
  int a.0_5;
  _Bool _7;

  <bb 2>:
  _2 = b.f;
  _3 = (int) _2;
  _4 = _3 > 0;
  _7 = _2 | _4;
  a.0_5 = (int) _7;
  a = a.0_5;
  return a.0_5;

op here is _2 (and other range's op is _4), the two range tests have been
successfully merged and the code chose to use the first one to be the one
that is kept.  Normally the range test isn't a SSA_NAME itself, but say
comparison etc. and thus emitting the statements before the statement
is desirable, but in this case we can't emit the new statements before it,
as they use _2.  Fixed thusly, bootstrapped/regtested on x86_64-linux and
i686-linux, ok for trunk and 4.8 (for the latter the fix is tiny bit
different, therefore attached)?

2013-12-06  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/59388
	* tree-ssa-reassoc.c (update_range_test): If op == range->exp,
	gimplify tem after stmt rather than before it.

	* gcc.c-torture/execute/pr59388.c: New test.


	Jakub
2013-12-06  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/59388
	* tree-ssa-reassoc.c (update_range_test): If op == range->exp,
	gimplify tem after stmt rather than before it.

	* gcc.c-torture/execute/pr59388.c: New test.

--- gcc/tree-ssa-reassoc.c.jj	2013-09-09 19:10:19.971488442 +0200
+++ gcc/tree-ssa-reassoc.c	2013-12-06 17:56:01.288842668 +0100
@@ -1980,8 +1980,15 @@ update_range_test (struct range_entry *r
 
   tem = fold_convert_loc (loc, optype, tem);
   gsi = gsi_for_stmt (stmt);
-  tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
-				  GSI_SAME_STMT);
+  /* In rare cases range->exp can be equal to lhs of stmt.
+     In that case we have to insert after the stmt rather then before
+     it.  */
+  if (op == range->exp)
+    tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, false,
+				    GSI_SAME_STMT);
+  else
+    tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
+				    GSI_SAME_STMT);
 
   /* If doing inter-bb range test optimization, update the
      stmts immediately.  Start with changing the first range test
--- gcc/testsuite/gcc.c-torture/execute/pr59388.c.jj	2013-12-06 17:41:55.811244282 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr59388.c	2013-12-06 17:32:28.000000000 +0100
@@ -0,0 +1,11 @@
+/* PR tree-optimization/59388 */
+
+int a;
+struct S { unsigned int f:1; } b;
+
+int
+main ()
+{
+  a = (0 < b.f) | b.f;
+  return a;
+}
Jeff Law - Dec. 6, 2013, 8:51 p.m.
On 12/06/13 13:33, Jakub Jelinek wrote:
> Hi!
>
> range->exp can in some cases be equal to op, as in the following testcase:
>    <unnamed-unsigned:1> _2;
>    int _3;
>    _Bool _4;
>    int a.0_5;
>    _Bool _7;
>
>    <bb 2>:
>    _2 = b.f;
>    _3 = (int) _2;
>    _4 = _3 > 0;
>    _7 = _2 | _4;
>    a.0_5 = (int) _7;
>    a = a.0_5;
>    return a.0_5;
>
> op here is _2 (and other range's op is _4), the two range tests have been
> successfully merged and the code chose to use the first one to be the one
> that is kept.  Normally the range test isn't a SSA_NAME itself, but say
> comparison etc. and thus emitting the statements before the statement
> is desirable, but in this case we can't emit the new statements before it,
> as they use _2.  Fixed thusly, bootstrapped/regtested on x86_64-linux and
> i686-linux, ok for trunk and 4.8 (for the latter the fix is tiny bit
> different, therefore attached)?
I was going to ask how this was possible, but then I looked at the 
testcase...  Sigh.


>
> 2013-12-06  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/59388
> 	* tree-ssa-reassoc.c (update_range_test): If op == range->exp,
> 	gimplify tem after stmt rather than before it.
>
> 	* gcc.c-torture/execute/pr59388.c: New test.
OK for the trunk.  Branch maintainers have final say for the branches.

jeff

Patch

--- gcc/tree-ssa-reassoc.c.jj	2013-11-23 15:21:23.000000000 +0100
+++ gcc/tree-ssa-reassoc.c	2013-12-06 17:27:01.908908702 +0100
@@ -2072,9 +2072,19 @@  update_range_test (struct range_entry *r
 
   tem = fold_convert_loc (loc, optype, tem);
   gsi = gsi_for_stmt (stmt);
-  tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
-				  GSI_SAME_STMT);
-  for (gsi_prev (&gsi); !gsi_end_p (gsi); gsi_prev (&gsi))
+  /* In rare cases range->exp can be equal to lhs of stmt.
+     In that case we have to insert after the stmt rather then before
+     it.  */
+  if (op == range->exp)
+    tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, false,
+				    GSI_CONTINUE_LINKING);
+  else
+    {
+      tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
+				      GSI_SAME_STMT);
+      gsi_prev (&gsi);
+    }
+  for (; !gsi_end_p (gsi); gsi_prev (&gsi))
     if (gimple_uid (gsi_stmt (gsi)))
       break;
     else
--- gcc/testsuite/gcc.c-torture/execute/pr59388.c.jj	2013-12-06 17:41:55.811244282 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr59388.c	2013-12-06 17:32:28.000000000 +0100
@@ -0,0 +1,11 @@ 
+/* PR tree-optimization/59388 */
+
+int a;
+struct S { unsigned int f:1; } b;
+
+int
+main ()
+{
+  a = (0 < b.f) | b.f;
+  return a;
+}