As in other places we have to be careful to use FP modes to represent
the underlying bit representation of an object.  With x87 floating-point
types there are no load or store instructions that preserve this and
XFmode can have padding.

When SRA faces the situation that a field is accessed with multiple
effective types as happens for example for unions it generally
choses an integer type if available.  But in the case in the PR
there's an aggregate type or a floating-point type only and we end
up chosing the register type.

SRA deals with similar situations for bit-precision integer types
and adjusts the replacement type to one covering the size of the
object.  The following patch makes sure we do the same when the
replacement has float mode and there were possibly two ways the
object was accessed.  I've chosen to use bitwise_type_for_mode
in this case as done for example by memcpy folding to avoid
creating a unsigned:96 replacement type on i?86 where
sizeof(long double) is 12.  This means we can fail to find an
integer type for a replacement which slightly complicates the patch
and it causes the testcase to no longer be SRAed on i?86.

Bootstrapped on x86_64-unknown-linux-gnu, there is some fallout in
the testsuite I need to compare to a clean run.  Comments welcome.

Richard.

        PR tree-optimization/58416
        * tree-sra.cc (analyze_access_subtree): For FP mode replacements
        with multiple access paths use a bitwise type instead or fail
        if not available.

        * gcc.dg/torture/pr58416.c: New testcase.
---
 gcc/testsuite/gcc.dg/torture/pr58416.c | 32 ++++++++++++
 gcc/tree-sra.cc                        | 72 ++++++++++++++++++--------
 2 files changed, 83 insertions(+), 21 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/torture/pr58416.c

diff --git a/gcc/testsuite/gcc.dg/torture/pr58416.c 
b/gcc/testsuite/gcc.dg/torture/pr58416.c
new file mode 100644
index 00000000000..0922b0e7089
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr58416.c
@@ -0,0 +1,32 @@
+/* { dg-do run } */
+
+struct s {
+  char s[sizeof(long double)];
+};
+
+union u {
+  long double d;
+  struct s s;
+};
+
+int main()
+{
+  union u x = {0};
+#if __SIZEOF_LONG_DOUBLE__ == 16
+  x.s = (struct s){"xxxxxxxxxxxxxxxx"};
+#elif __SIZEOF_LONG_DOUBLE__ == 12
+  x.s = (struct s){"xxxxxxxxxxxx"};
+#elif __SIZEOF_LONG_DOUBLE__ == 8
+  x.s = (struct s){"xxxxxxxx"};
+#elif __SIZEOF_LONG_DOUBLE__ == 4
+  x.s = (struct s){"xxxx"};
+#endif
+
+  union u y = x;
+
+  for (unsigned char *p = (unsigned char *)&y + sizeof y;
+       p-- > (unsigned char *)&y;)
+    if (*p != (unsigned char)'x')
+      __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
index 8040b0c5645..bc9a7b3ee04 100644
--- a/gcc/tree-sra.cc
+++ b/gcc/tree-sra.cc
@@ -2868,40 +2868,70 @@ analyze_access_subtree (struct access *root, struct 
access *parent,
       /* Always create access replacements that cover the whole access.
          For integral types this means the precision has to match.
         Avoid assumptions based on the integral type kind, too.  */
-      if (INTEGRAL_TYPE_P (root->type)
-         && ((TREE_CODE (root->type) != INTEGER_TYPE
-              && TREE_CODE (root->type) != BITINT_TYPE)
-             || TYPE_PRECISION (root->type) != root->size)
-         /* But leave bitfield accesses alone.  */
-         && (TREE_CODE (root->expr) != COMPONENT_REF
-             || !DECL_BIT_FIELD (TREE_OPERAND (root->expr, 1))))
+      if ((INTEGRAL_TYPE_P (root->type)
+          && ((TREE_CODE (root->type) != INTEGER_TYPE
+               && TREE_CODE (root->type) != BITINT_TYPE)
+              || TYPE_PRECISION (root->type) != root->size)
+          /* But leave bitfield accesses alone.  */
+          && (TREE_CODE (root->expr) != COMPONENT_REF
+              || !DECL_BIT_FIELD (TREE_OPERAND (root->expr, 1))))
+         /* Avoid a floating-point replacement when there's multiple
+            ways this field is accessed.   On some targets this can
+            cause correctness issues, see PR58416.  */
+         || (FLOAT_MODE_P (TYPE_MODE (root->type))
+             && !root->grp_same_access_path))
        {
          tree rt = root->type;
          gcc_assert ((root->offset % BITS_PER_UNIT) == 0
                      && (root->size % BITS_PER_UNIT) == 0);
          if (TREE_CODE (root->type) == BITINT_TYPE)
            root->type = build_bitint_type (root->size, TYPE_UNSIGNED (rt));
+         else if (FLOAT_MODE_P (TYPE_MODE (root->type)))
+           {
+             tree bt = bitwise_type_for_mode (TYPE_MODE (root->type));
+             if (!bt)
+               {
+                 if (dump_file && (dump_flags & TDF_DETAILS))
+                   {
+                     fprintf (dump_file, "Failed to change the type of a "
+                              "replacement for ");
+                     print_generic_expr (dump_file, root->base);
+                     fprintf (dump_file, " offset: %u, size: %u ",
+                              (unsigned) root->offset, (unsigned) root->size);
+                     fprintf (dump_file, " to an integer.\n");
+                   }
+                 allow_replacements = false;
+               }
+             else
+               root->type = bt;
+           }
          else
            root->type = build_nonstandard_integer_type (root->size,
                                                         TYPE_UNSIGNED (rt));
-         root->expr = build_ref_for_offset (UNKNOWN_LOCATION, root->base,
-                                            root->offset, root->reverse,
-                                            root->type, NULL, false);
-
-         if (dump_file && (dump_flags & TDF_DETAILS))
+         if (allow_replacements)
            {
-             fprintf (dump_file, "Changing the type of a replacement for ");
-             print_generic_expr (dump_file, root->base);
-             fprintf (dump_file, " offset: %u, size: %u ",
-                      (unsigned) root->offset, (unsigned) root->size);
-             fprintf (dump_file, " to an integer.\n");
+             root->expr = build_ref_for_offset (UNKNOWN_LOCATION, root->base,
+                                                root->offset, root->reverse,
+                                                root->type, NULL, false);
+
+             if (dump_file && (dump_flags & TDF_DETAILS))
+               {
+                 fprintf (dump_file, "Changing the type of a replacement for 
");
+                 print_generic_expr (dump_file, root->base);
+                 fprintf (dump_file, " offset: %u, size: %u ",
+                          (unsigned) root->offset, (unsigned) root->size);
+                 fprintf (dump_file, " to an integer.\n");
+               }
            }
        }
 
-      root->grp_to_be_replaced = 1;
-      root->replacement_decl = create_access_replacement (root);
-      sth_created = true;
-      hole = false;
+      if (allow_replacements)
+       {
+         root->grp_to_be_replaced = 1;
+         root->replacement_decl = create_access_replacement (root);
+         sth_created = true;
+         hole = false;
+       }
     }
   else
     {
-- 
2.43.0

Reply via email to