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)

Reply via email to