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.
* tree.h (storage_order_barrier_p): Also return false if the
operand of the VIEW_CONVERT_EXPR has reverse storage order.
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
-- { dg-do run }
-- { dg-options "-O2" }
with SSO19_Pkg; use SSO19_Pkg;
procedure SSO19 is
R : constant Rec := (D => (I => 8, F => 4.6095713E-41));
begin
if not Is_Valid (R) then
raise Program_Error;
end if;
end;
with System;
package SSO19_Pkg is
subtype Small_Int is Short_Integer range -1000 .. 1000;
type Data is record
I : Small_Int;
F : Float;
end record;
for Data use record
I at 0 range 0 .. 15;
F at 4 range 0 .. 31;
end record;
for Data'Bit_Order use System.High_Order_First;
for Data'Scalar_Storage_Order use System.High_Order_First;
type Rec is record
D : Data;
end record;
function Is_Valid (Item : Rec) return Boolean;
end SSO19_Pkg;
package body SSO19_Pkg is
function Is_Valid_Private (Item : Data) return Boolean is
begin
return Item.I'Valid and Item.F'Valid;
end Is_Valid_Private;
function Is_Valid (Item : Rec) return Boolean is
begin
return Is_Valid_Private (Item.D);
end Is_Valid;
end SSO19_Pkg;