Hi,
On Wed, May 21 2025, Eric Botcazou wrote:
> Hi,
>
> IPA-SRA generally works fine in the presence of reverse Scalar_Storage_Order
> by propagating the relevant flag onto the newly generated MEM_REFs. However
> we have been recently faced with a specific Ada pattern that it doesn't
> handle
> correctly: 'Valid applied to a floating-point component of an aggregate type
> with reverse Scalar_Storage_Order.
>
> The attribute is implemented by a call to a specific routine of the runtime
> that expects a pointer to the object so, in the case of a component with
> reverse SSO, the compiler first loads it from the aggregate to get back the
> native storage order, but it does the load using an array of bytes instead of
> the floating-point type to prevent the FPU from fiddling with the value,
> which
> yields in the .original dump file:
>
> *(character[1:4] *) &F2b = VIEW_CONVERT_EXPR<character[1:4]>(item.f);
>
> Of course that's a bit convoluted, but it does not seem that another method
> would be simpler or even work, and using VIEW_CONVERT_EXPR to toggle the SSO
> is supposed to be supported in any case (unlike aliasing or type punning).
>
> The attached patch makes it work. While the call to storage_order_barrier_p
> from IPA-SRA is quite natural (the regular SRA has it too), the tweak to the
> predicate itself is needed to handle the scalar->aggregate conversion, which
> is admittedly awkward but again without clear alternative.
>
> Tested on x86-64/Linux, OK for the mainline and 15 branch? Technically, this
> is a regression in GCC 10.x and later, but the pattern is so specific, even
> in
> Ada, that patching earlier branches does not seem worth the hassle.
>
>
> 2025-05-21 Eric Botcazou <ebotca...@adacore.com>
>
> * ipa-sra.cc (scan_expr_access): Also disqualify storage order
> barriers from splitting.
The IPA-SRA change is OK.
> * tree.h (storage_order_barrier_p): Also return false if the
> operand of the VIEW_CONVERT_EXPR has reverse storage order.
I cannot approve this one (but FWIW it looks OKish to me too).
Thanks,
Martin
>
>
> 2025-05-21 Eric Botcazou <ebotca...@adacore.com>
>
> * gnat.dg/sso19.adb: New test.
> * gnat.dg/sso19_pkg.ads, gnat.dg/sso19_pkg.adb: New helper.
>
> --
> Eric Botcazou
> diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc
> index 88bfae9502c..6e6cf895988 100644
> --- a/gcc/ipa-sra.cc
> +++ b/gcc/ipa-sra.cc
> @@ -1848,6 +1848,12 @@ scan_expr_access (tree expr, gimple *stmt,
> isra_scan_context ctx,
> if (!desc || !desc->split_candidate)
> return;
>
> + if (storage_order_barrier_p (expr))
> + {
> + disqualify_split_candidate (desc, "Encountered a storage order
> barrier.");
> + return;
> + }
> +
> if (!poffset.is_constant (&offset)
> || !psize.is_constant (&size)
> || !pmax_size.is_constant (&max_size))
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 99f26177628..1e41316b4c9 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -5499,7 +5499,7 @@ storage_order_barrier_p (const_tree t)
> && TYPE_REVERSE_STORAGE_ORDER (TREE_TYPE (op)))
> return true;
>
> - return false;
> + return reverse_storage_order_for_component_p (op);
> }
>
> /* Given a DECL or TYPE, return the scope in which it was declared, or
