diff mbox

[PR72835] Incorrect arithmetic optimization involving bitfield arguments

Message ID 0a1eaaf8-3ede-cd56-ffb5-40b25f94e46e@linaro.org
State Superseded
Headers show

Commit Message

Kugan Vivekanandarajah Aug. 9, 2016, 1:43 p.m. UTC 
Hi,

The test-case in PR72835 is failing with -O2 and passing with 
-fno-tree-reassoc. It also passes with -O2 -fno-tree-vrp.

diff of .115t.dse2 and .116t.reassoc1 for the c++ testcase is as 
follows, which looks OK.

+  unsigned int _16;
+  unsigned int _17;
+  unsigned int _18;

    <bb 2>:
    _1 = s1.m2;
    _2 = (unsigned int) _1;
    _3 = s1.m3;
    _4 = (unsigned int) _3;
-  _5 = -_4;
-  _6 = _2 * _5;
+  _5 = _4;
+  _6 = _5 * _2;
    var_32.0_7 = var_32;
    _8 = (unsigned int) var_32.0_7;
    _9 = s1.m1;
    _10 = (unsigned int) _9;
-  _11 = -_10;
-  _12 = _8 * _11;
-  c_14 = _6 + _12;
+  _11 = _10;
+  _12 = _11 * _8;
+  _16 = _12 + _6;
+  _18 = _16;
+  _17 = -_18;
+  c_14 = _17;
    if (c_14 != 4098873984)


However, I noticed that when we re-associate and assign different 
operands to the stmts, we are not resetting the flow information to the 
LHS. This looks wrong. Attached patch resets it. With this, the 
testcases in TH PR is now working.


Bootstrap and regression testing for x86_64-linux-gnu is in progress. Is 
this OK for trunk if there is no regression.

Thanks,
Kugan

gcc/testsuite/ChangeLog:

2016-08-09  Kugan Vivekanandarajah  <kuganv@linaro.org>

	PR tree-optimization/72835
	* gcc.dg/torture/pr72835.c: New test.

gcc/ChangeLog:

2016-08-09  Kugan Vivekanandarajah  <kuganv@linaro.org>

	PR tree-optimization/72835
	* tree-ssa-reassoc.c (rewrite_expr_tree): Reset value_range of LHS when
	operands are changed.
	(rewrite_expr_tree_parallel): Likewise.
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/torture/pr72835.c b/gcc/testsuite/gcc.dg/torture/pr72835.c
index e69de29..d7d0a8d 100644
--- a/gcc/testsuite/gcc.dg/torture/pr72835.c
+++ b/gcc/testsuite/gcc.dg/torture/pr72835.c
@@ -0,0 +1,36 @@ 
+/* PR tree-optimization/72835 */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+struct struct_1 {
+    unsigned int m1 : 6 ;
+    unsigned int m2 : 24 ;
+    unsigned int m3 : 6 ;
+};
+
+unsigned short var_32 = 0x2d10;
+
+struct struct_1 s1;
+
+void init ()
+{
+  s1.m1 = 4;
+  s1.m2 = 0x7ca4b8;
+  s1.m3 = 24;
+}
+
+void foo ()
+{
+  unsigned int c =
+    ((unsigned int) s1.m2) * (-((unsigned int) s1.m3))
+    + (var_32) * (-((unsigned int) (s1.m1)));
+  if (c != 4098873984)
+    __builtin_abort ();
+}
+
+int main ()
+{
+    init ();
+    foo ();
+    return 0;
+}
diff --git a/gcc/tree-ssa-reassoc.c b/gcc/tree-ssa-reassoc.c
index 7fd7550..b864ed1 100644
--- a/gcc/tree-ssa-reassoc.c
+++ b/gcc/tree-ssa-reassoc.c
@@ -3945,6 +3945,7 @@  rewrite_expr_tree (gimple *stmt, unsigned int opindex,
 	      gimple_assign_set_rhs1 (stmt, oe1->op);
 	      gimple_assign_set_rhs2 (stmt, oe2->op);
 	      update_stmt (stmt);
+	      reset_flow_sensitive_info (lhs);
 	    }
 
 	  if (rhs1 != oe1->op && rhs1 != oe2->op)
@@ -4193,9 +4194,11 @@  rewrite_expr_tree_parallel (gassign *stmt, int width,
 	 others are recreated.  */
       if (i == stmt_num - 1)
 	{
+	  tree lhs = gimple_assign_lhs (stmts[i]);
 	  gimple_assign_set_rhs1 (stmts[i], op1);
 	  gimple_assign_set_rhs2 (stmts[i], op2);
 	  update_stmt (stmts[i]);
+	  reset_flow_sensitive_info (lhs);
 	}
       else
 	{