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