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)
