On Mon, 19 Aug 2024, Martin Jambor wrote: > Hi, > > PR 58416 shows that storing non-floating point data to floating point > scalar registers can lead to miscompilations when the data is > normalized or otherwise processed upon loading to a register. To > avoid that risk, this patch detects situations where we have multiple > types and a we decide to represent the data in a type with a mode that > is known to not be able to transfer actual bits reliably using the new > TARGET_MODE_CAN_TRANSFER_BITS hook. > > Bootstrapped and tested on x86_64-linux. OK for trunk?
OK (well, you know SRA best). > Any back-ports to release branches would of course need a back-port of > the hook itself, unfortunately. Of course. Thanks, Richard. > Thanks, > > Martin > > > gcc/ChangeLog: > > 2024-08-19 Martin Jambor <mjam...@suse.cz> > > PR target/58416 > * tree-sra.cc (types_risk_mangled_binary_repr_p): New function. > (sort_and_splice_var_accesses): Use it. > (propagate_subaccesses_from_rhs): Likewise. > > gcc/testsuite/ChangeLog: > > 2024-08-19 Martin Jambor <mjam...@suse.cz> > > PR target/58416 > * gcc.dg/torture/pr58416.c: New test. > --- > gcc/testsuite/gcc.dg/torture/pr58416.c | 32 ++++++++++++++++++++++++++ > gcc/tree-sra.cc | 28 +++++++++++++++++++++- > 2 files changed, 59 insertions(+), 1 deletion(-) > 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..64e2f007d68 100644 > --- a/gcc/tree-sra.cc > +++ b/gcc/tree-sra.cc > @@ -2335,6 +2335,19 @@ same_access_path_p (tree exp1, tree exp2) > return true; > } > > +/* Return true when either T1 is a type that, when loaded into a register and > + stored back to memory will yield the same bits or when both T1 and T2 are > + compatible. */ > + > +static bool > +types_risk_mangled_binary_repr_p (tree t1, tree t2) > +{ > + if (mode_can_transfer_bits (TYPE_MODE (t1))) > + return false; > + > + return !types_compatible_p (t1, t2); > +} > + > /* Sort all accesses for the given variable, check for partial overlaps and > return NULL if there are any. If there are none, pick a representative > for > each combination of offset and size and create a linked list out of them. > @@ -2461,6 +2474,17 @@ sort_and_splice_var_accesses (tree var) > } > unscalarizable_region = true; > } > + else if (types_risk_mangled_binary_repr_p (access->type, ac2->type)) > + { > + if (dump_file && (dump_flags & TDF_DETAILS)) > + { > + fprintf (dump_file, "Cannot scalarize the following access " > + "because data would be held in a mode which is not " > + "guaranteed to preserve all bits.\n "); > + dump_access (dump_file, access, false); > + } > + unscalarizable_region = true; > + } > > if (grp_same_access_path > && !same_access_path_p (access->expr, ac2->expr)) > @@ -3127,7 +3151,9 @@ propagate_subaccesses_from_rhs (struct access *lacc, > struct access *racc) > ret = true; > subtree_mark_written_and_rhs_enqueue (lacc); > } > - if (!lacc->first_child && !racc->first_child) > + if (!lacc->first_child > + && !racc->first_child > + && !types_risk_mangled_binary_repr_p (racc->type, lacc->type)) > { > /* We are about to change the access type from aggregate to scalar, > so we need to put the reverse flag onto the access, if any. */ > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)