tree-optimization: do not lose reverse storage order when translating a ref
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap |
success
|
Build passed
|
Commit Message
When vn_reference_lookup_3 translates a reference through an aggregate
copy it may fail to find a common base and instead reduce the lookup to
the base of the copy's right-hand side plus a constant offset, folding
all of the original operands away. A reversed storage order is a
property of the component and not of its position, so it cannot be
recovered from that offset: the translated reference ends up in natural
order and the later "assignment from a constant" case interprets the
stored bytes with the wrong bit numbering, silently producing a wrong
value.
Punt in that case, i.e. when no original operand survives and either
access is in reverse storage order. When an operand does survive it
stays outermost and carries the flag, so the storage order of the
translated reference is unchanged; a reverse flag on an inner operand
only describes an inner container and does not affect the byte order of
the scalar access.
reverse_storage_order_for_component_p now takes its argument by const
reference so that the auto_vec holding the right-hand side operands can
be passed to it.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Eric, is the SSO use sound for the testcase (which was LLM generated)?
Thanks,
Richard.
* tree-ssa-sccvn.cc (reverse_storage_order_for_component_p): Take
the operands by const reference.
(vn_reference_lookup_3): Punt when translating a reference through
an aggregate copy consumes all of its operands and either access
has reverse storage order.
* gcc.dg/torture/sso-fre-1.c: New test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
gcc/testsuite/gcc.dg/torture/sso-fre-1.c | 43 ++++++++++++++++++++++++
gcc/tree-ssa-sccvn.cc | 13 ++++++-
2 files changed, 55 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/sso-fre-1.c
Comments
> Eric, is the SSO use sound for the testcase (which was LLM generated)?
IIUC it writes into a big-endian structure using a little-endian character, so
the (implicit) bit ordering is changed; I guess that a memcpy would also work?
On Thu, 3 Sep 2026, Eric Botcazou wrote:
> > Eric, is the SSO use sound for the testcase (which was LLM generated)?
>
> IIUC it writes into a big-endian structure using a little-endian character, so
> the (implicit) bit ordering is changed; I guess that a memcpy would also work?
To get the data flow yes, but of course the FRE issue is that
the SSO attribution is lost and we generate wrong code, so actual
SSO attribution is needed for that.
I was mostly questioning SSO on bitfields given bitfield layout might
be special. I suppose avoiding padding in S0 might make this more
portable (so signed/unsigned char for f7/f6). The testcase fails
with that as well before the fix.
Richard.
> To get the data flow yes, but of course the FRE issue is that
> the SSO attribution is lost and we generate wrong code, so actual
> SSO attribution is needed for that.
Sure, but we do not support type punning when it toggles the storage order and
this testcase does that (admittedly in a more subtle way than most cases, as
you need to realize that the bit order is implicitly changed for bitfields).
What needs to be prevented is the optimization of:
g.f3 = { 1, 4 };
into
*(char *) &g.f3 = ...
because the SSO attribution is indeed lost.
> I was mostly questioning SSO on bitfields given bitfield layout might
> be special. I suppose avoiding padding in S0 might make this more
> portable (so signed/unsigned char for f7/f6). The testcase fails
> with that as well before the fix.
SSO on bitfields was the motivating case in Ada though.
On Thu, 3 Sep 2026, Eric Botcazou wrote:
> > To get the data flow yes, but of course the FRE issue is that
> > the SSO attribution is lost and we generate wrong code, so actual
> > SSO attribution is needed for that.
>
> Sure, but we do not support type punning when it toggles the storage order and
> this testcase does that (admittedly in a more subtle way than most cases, as
> you need to realize that the bit order is implicitly changed for bitfields).
Hmm, does it? You mean
/* Reversed bit order, so this gives f7 == 1 and f6 == 4. */
*(char *) &g.f3 = 0x24;
? But this is just
memset (&g.f3, 1, 0x23);
as it writes a single byte via a character type there is no storage
order involved?
The
q = (struct S0 *) ((char *) &l + __builtin_offsetof (struct S1, f3));
is just a way to get an offsetted MEM_EXPR I guess, it's also not
punning.
> What needs to be prevented is the optimization of:
>
> g.f3 = { 1, 4 };
>
> into
>
> *(char *) &g.f3 = ...
>
> because the SSO attribution is indeed lost.
I think what happened is that
return q->f7;
gets re-written through
l = g;
and that then picks up the *(char *) &g.f3 store, eliding the SSO
of the q->f7 access.
> > I was mostly questioning SSO on bitfields given bitfield layout might
> > be special. I suppose avoiding padding in S0 might make this more
> > portable (so signed/unsigned char for f7/f6). The testcase fails
> > with that as well before the fix.
>
> SSO on bitfields was the motivating case in Ada though.
I see. I have simplified S0 by using 'char' as type for the bitfields,
avoiding any bit padding.
Do you think this is a non-bug that is fixed? If not I plan to push it.
Thanks,
Richard.
new file mode 100644
@@ -0,0 +1,43 @@
+/* { dg-do run } */
+/* { dg-require-effective-target le } */
+
+/* Value numbering used to translate a reference through an aggregate copy
+ by folding the components of the reference into a constant offset into
+ the right-hand side of the copy. A reversed storage order is a property
+ of the component and not of its position, so it was lost in the process
+ and the load of F7 below was resolved to the least significant bits of
+ the byte instead of to its most significant bits. */
+
+struct __attribute__((scalar_storage_order("big-endian"))) S0
+{
+ signed f7 : 3;
+ unsigned f6 : 5;
+};
+
+struct S1 { int a; struct S0 f3; char pad; short s; };
+
+struct S1 g;
+struct S1 *escape;
+
+__attribute__((noipa)) int
+foo (void)
+{
+ struct S1 l;
+ struct S0 *q;
+
+ /* Reversed bit order, so this gives f7 == 1 and f6 == 4. */
+ *(char *) &g.f3 = 0x24;
+ l = g;
+ /* Keep L addressable so that it is not scalarized away. */
+ escape = &l;
+ q = (struct S0 *) ((char *) &l + __builtin_offsetof (struct S1, f3));
+ return q->f7;
+}
+
+int
+main (void)
+{
+ if (foo () != 1)
+ __builtin_abort ();
+ return 0;
+}
@@ -1727,7 +1727,7 @@ contains_storage_order_barrier_p (vec<vn_reference_op_s> ops)
/* Return true if OPS represent an access with reverse storage order. */
static bool
-reverse_storage_order_for_component_p (vec<vn_reference_op_s> ops)
+reverse_storage_order_for_component_p (const vec<vn_reference_op_s> &ops)
{
unsigned i = 0;
if (ops[i].opcode == REALPART_EXPR || ops[i].opcode == IMAGPART_EXPR)
@@ -3779,6 +3779,17 @@ vn_reference_lookup_3 (ao_ref *ref, tree vuse, void *data_,
tree rhs1 = gimple_assign_rhs1 (def_stmt);
copy_reference_ops_from_ref (rhs1, &rhs);
+ /* When none of the original operands survives the storage order of
+ the translated reference is the one of the RHS of the copy. The
+ operands we folded into a constant offset above may well have
+ specified a reverse storage order, which is a property of the
+ component and not of its position, so it is not recoverable from
+ that offset. Punt unless both accesses are in natural order. */
+ if (i < 0
+ && (reverse_storage_order_for_component_p (vr->operands)
+ || reverse_storage_order_for_component_p (rhs)))
+ return (void *)-1;
+
/* Apply an extra offset to the inner MEM_REF of the RHS. */
bool force_no_tbaa = false;
if (maybe_ne (extra_off, 0))