]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
sra-9: Fix sra_modify_expr handling of partial writes (PR 94482)
authorMartin Jambor <mjambor@suse.cz>
Tue, 21 Apr 2020 12:20:37 +0000 (14:20 +0200)
committerMartin Jambor <mjambor@suse.cz>
Tue, 21 Apr 2020 12:20:37 +0000 (14:20 +0200)
This is a fairly straightforward backport of the mainline fix for 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 part
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 converting.  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.
This is the only difference from the mainline patch which has two
hunks, but the code handling BIT_FIELD_REF is not present in gcc-9.

Oh, and the testcase options were changed to what Jakub put there on
the mainline to suppress all vector ABI warnings.

Bootstrapped and tested on x86_64-linux.

2020-04-21  Martin Jambor  <mjambor@suse.cz>

        Backport from master
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.

gcc/ChangeLog
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/torture/pr94482.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c [new file with mode: 0644]
gcc/tree-sra.c
gcc/tree-ssa-forwprop.c

index 93bd849accace81ec5a4cb9a4e4c7ca3ed91bb0c..4cb1497a61f215bd91a8fd4ece06d64097a9d7fc 100644 (file)
@@ -1,3 +1,18 @@
+2020-04-21  Martin Jambor  <mjambor@suse.cz>
+
+       Backport from master
+       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-20  H.J. Lu  <hongjiu.lu@intel.com>
 
        Backport from master
index fb039ee0e97a596b70daf36f7d81c9207d01787f..74a3cdf7b80d25947ca697574fea0f06a59679d0 100644 (file)
@@ -1,3 +1,12 @@
+2020-04-21  Martin Jambor  <mjambor@suse.cz>
+
+       Backport from master
+       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-20  Harald Anlauf  <anlauf@gmx.de>
 
        Backport from mainline.
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..9264842
--- /dev/null
@@ -0,0 +1,36 @@
+/* { dg-do run } */
+/* { dg-additional-options "-Wno-psabi -w" } */
+/* { dg-additional-options "-msse2" { target sse2_runtime } } */
+
+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
@@ -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;
+}
index 0a5c24a183a7240bb073e2ea69398c07dadb4560..805839cac503ba2b188034165c23091b5ab56122 100644 (file)
@@ -2329,7 +2329,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");
        }
     }
@@ -3187,6 +3187,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)
     {
@@ -3197,7 +3198,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;
@@ -3225,13 +3229,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;
 
index bbfa1bc6fae1f62c5bd45f03187e05f26e3b5d8e..ca5443b79b3dfd03a14c29228a203109cc28166e 100644 (file)
@@ -2357,7 +2357,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;