sra: Fix sra_modify_expr handling of partial writes (PR 94482)

when sra_modify_expr is invoked on an expression that modifies only
part of the underlying replacement, such as a BIT_FIELD_REF on a LHS
of an assignment and the SRA replacement's type is not compatible with
what is being replaced (0th operand of the B_F_R in the above
example), it does not work properly, basically throwing away the partd
of the expr that should have stayed intact.

This is fixed in two ways.  For BIT_FIELD_REFs, which operate on the
binary image of the replacement (and so in a way serve as a
VIEW_CONVERT_EXPR) we just do not bother with convertsing.  For
REALPART_EXPRs and IMAGPART_EXPRs, if the replacement is not a
register, we insert a VIEW_CONVERT_EXPR under
the complex partial access expression, which is always OK, for loads
from registers we take the extra step of converting it to a temporary.

This revealed a bug in fwprop which is fixed with the hunk from Richi.

The testcase for handling REALPART_EXPR and IMAGPART_EXPR is a bit
fragile because SRA prefers complex and vector types over anything
else (and in between the two it decides based on TYPE_UID which in my
testing today always preferred complex types) and so I only run it at
-O1 (which is the only level where the the test fails for me).

Bootstrapped and tested on x86_64-linux, i686-linux and aarch64-linux.

2020-04-09  Martin Jambor  <mjambor@suse.cz>
	    Richard Biener  <rguenther@suse.de>

	PR tree-optimization/94482
	* tree-sra.c (create_access_replacement): Dump new replacement with
	TDF_UID.
	(sra_modify_expr): Fix handling of cases when the original EXPR writes
	to only part of the replacement.
	* tree-ssa-forwprop.c (pass_forwprop::execute): Properly verify
	the first operand of combinations into REAL/IMAGPART_EXPR and
	BIT_FIELD_REF.

	testsuite/
	* gcc.dg/torture/pr94482.c: New test.
	* gcc.dg/tree-ssa/pr94482-2.c: Likewise.

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index d211d7391a836761fc2aa0174ccc0822618f58dc..368dfd6094e08b0a58cbf3888a9375bbc2bba755 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,15 @@
+2020-04-09  Martin Jambor  <mjambor@suse.cz>
+           Richard Biener  <rguenther@suse.de>
+
+       PR tree-optimization/94482
+       * tree-sra.c (create_access_replacement): Dump new replacement with
+       TDF_UID.
+       (sra_modify_expr): Fix handling of cases when the original EXPR writes
+       to only part of the replacement.
+       * tree-ssa-forwprop.c (pass_forwprop::execute): Properly verify
+       the first operand of combinations into REAL/IMAGPART_EXPR and
+       BIT_FIELD_REF.
+
 2020-04-09  Richard Sandiford  <richard.sandiford@arm.com>
 
        * doc/sourcebuild.texi (check-function-bodies): Treat the third

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index a5bd5614e5adff77c8c0c8bca2cc012cadb867c2..e01fdb0fe2c7f0102b72f9c7462f588e4765f30c 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2020-04-09  Martin Jambor  <mjambor@suse.cz>
+
+       PR tree-optimization/94482
+       * gcc.dg/torture/pr94482.c: New test.
+       * gcc.dg/tree-ssa/pr94482-2.c: Likewise.
+
 2020-04-09  Marek Polacek  <polacek@redhat.com>
 
        PR c++/93790

diff --git a/gcc/testsuite/gcc.dg/torture/pr94482.c b/gcc/testsuite/gcc.dg/torture/pr94482.c
new file mode 100644 (file)
index 0000000..d9ccaf3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr94482.c
@@ -0,0 +1,36 @@
+/* { dg-do run } */
+/* { dg-options "-msse" { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-require-effective-target sse_runtime { target { i?86-*-* x86_64-*-* } } } */
+
+typedef unsigned V __attribute__ ((__vector_size__ (16)));
+union U
+{
+  V j;
+  unsigned long long i __attribute__ ((__vector_size__ (16)));
+};
+
+static inline __attribute__((always_inline)) V
+foo (unsigned long long a)
+{
+  union U z = { .j = (V) {} };
+  for (unsigned long i = 0; i < 1; i++)
+    z.i[i] = a;
+  return z.j;
+}
+
+static inline __attribute__((always_inline)) V
+bar (V a, unsigned long long i, int q)
+{
+  union U z = { .j = a };
+  z.i[q] = i;
+  return z.j;
+}
+
+int
+main ()
+{
+  union U z = { .j = bar (foo (1729), 2, 1) };
+  if (z.i[0] != 1729)
+    __builtin_abort ();
+  return 0;
+}

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c
new file mode 100644 (file)
index 0000000..fcac9d5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c
@@ -0,0 +1,50 @@
+/* { dg-do run } */
+/* { dg-options "-O1" } */
+
+typedef unsigned long V __attribute__ ((__vector_size__ (8)));
+typedef _Complex int Ci;
+typedef _Complex float Cf;
+
+union U
+{
+  Ci ci;
+  Cf cf;
+};
+
+volatile Ci vgi;
+
+Cf foo (Cf c)
+{
+  __real c = 0x1ffp10;
+  return c;
+}
+
+Ci ioo (Ci c)
+{
+  __real c = 50;
+  return c;
+}
+
+
+int main (int argc, char *argv[])
+{
+  union U u;
+
+  __real u.ci = 500;
+  __imag u.ci = 1000;
+  vgi = u.ci;
+
+  u.ci = ioo (u.ci);
+  __imag u.ci = 100;
+
+  if (__real u.ci != 50 || __imag u.ci != 100)
+    __builtin_abort();
+
+  u.cf = foo (u.cf);
+  __imag u.cf = 0x1p3;
+
+  if (__real u.cf != 0x1ffp10 || __imag u.cf != 0x1p3)
+    __builtin_abort();
+
+  return 0;
+}

diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index b2056b5875030dcda14c625d045141883d9397d7..84c113c403cc5d42afbc018773f61ea1d9a4ee55 100644 (file)
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -2257,7 +2257,7 @@ create_access_replacement (struct access *access, tree reg_type = NULL_TREE)
          print_generic_expr (dump_file, access->base);
          fprintf (dump_file, " offset: %u, size: %u: ",
                   (unsigned) access->offset, (unsigned) access->size);
-         print_generic_expr (dump_file, repl);
+         print_generic_expr (dump_file, repl, TDF_UID);
          fprintf (dump_file, "\n");
        }
     }
@@ -3698,6 +3698,7 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gsi, bool write)
   location_t loc;
   struct access *access;
   tree type, bfr, orig_expr;
+  bool partial_cplx_access = false;
 
   if (TREE_CODE (*expr) == BIT_FIELD_REF)
     {
@@ -3708,7 +3709,10 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gsi, bool write)
     bfr = NULL_TREE;
 
   if (TREE_CODE (*expr) == REALPART_EXPR || TREE_CODE (*expr) == IMAGPART_EXPR)
-    expr = &TREE_OPERAND (*expr, 0);
+    {
+      expr = &TREE_OPERAND (*expr, 0);
+      partial_cplx_access = true;
+    }
   access = get_access_for_expr (*expr);
   if (!access)
     return false;
@@ -3736,13 +3740,32 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gsi, bool write)
          be accessed as a different type too, potentially creating a need for
          type conversion (see PR42196) and when scalarized unions are involved
          in assembler statements (see PR42398).  */
-      if (!useless_type_conversion_p (type, access->type))
+      if (!bfr && !useless_type_conversion_p (type, access->type))
        {
          tree ref;
 
          ref = build_ref_for_model (loc, orig_expr, 0, access, gsi, false);
 
-         if (write)
+         if (partial_cplx_access)
+           {
+           /* VIEW_CONVERT_EXPRs in partial complex access are always fine in
+              the case of a write because in such case the replacement cannot
+              be a gimple register.  In the case of a load, we have to
+              differentiate in between a register an non-register
+              replacement.  */
+             tree t = build1 (VIEW_CONVERT_EXPR, type, repl);
+             gcc_checking_assert (!write || access->grp_partial_lhs);
+             if (!access->grp_partial_lhs)
+               {
+                 tree tmp = make_ssa_name (type);
+                 gassign *stmt = gimple_build_assign (tmp, t);
+                 /* This is always a read. */
+                 gsi_insert_before (gsi, stmt, GSI_SAME_STMT);
+                 t = tmp;
+               }
+             *expr = t;
+           }
+         else if (write)
            {
              gassign *stmt;
 

diff --git a/gcc/tree-ssa-forwprop.c b/gcc/tree-ssa-forwprop.c
index e7eaa18ccad2486ec04eaf3a942f282db3d95702..3d8acf7eb0355601cdaaa81bd8760eab585cf82d 100644 (file)
--- a/gcc/tree-ssa-forwprop.c
+++ b/gcc/tree-ssa-forwprop.c
@@ -2815,7 +2815,8 @@ pass_forwprop::execute (function *fun)
                    continue;
                  if (!is_gimple_assign (use_stmt)
                      || (gimple_assign_rhs_code (use_stmt) != REALPART_EXPR
-                         && gimple_assign_rhs_code (use_stmt) != IMAGPART_EXPR))
+                         && gimple_assign_rhs_code (use_stmt) != IMAGPART_EXPR)
+                     || TREE_OPERAND (gimple_assign_rhs1 (use_stmt), 0) != lhs)
                    {
                      rewrite = false;
                      break;
@@ -2877,7 +2878,8 @@ pass_forwprop::execute (function *fun)
                  if (is_gimple_debug (use_stmt))
                    continue;
                  if (!is_gimple_assign (use_stmt)
-                     || gimple_assign_rhs_code (use_stmt) != BIT_FIELD_REF)
+                     || gimple_assign_rhs_code (use_stmt) != BIT_FIELD_REF
+                     || TREE_OPERAND (gimple_assign_rhs1 (use_stmt), 0) != lhs)
                    {
                      rewrite = false;
                      break;