Re: [PR72835] Incorrect arithmetic optimization involving bitfield arguments

[kugan <kugan.vivekanandarajah@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]



On 09/08/16 23:43, kugan wrote:
> 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.

There was no new regression while testing. I also moved the testcase 
from gcc.dg/torture/pr72835.c to gcc.dg/tree-ssa/pr72835.c. Is this OK 
for trunk?

Thanks,
Kugan

gcc/testsuite/ChangeLog:

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

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

gcc/ChangeLog:

2016-08-10  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.

[-- Attachment #2: pr72835.txt --]
[-- Type: text/plain, Size: 1613 bytes --]

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr72835.c b/gcc/testsuite/gcc.dg/tree-ssa/pr72835.c
index e69de29..d7d0a8d 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr72835.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/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
 	{