public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/113282] New: RISC-V non-atomic union store/load reordering
@ 2024-01-08 21:25 patrick at rivosinc dot com
2024-01-08 22:13 ` [Bug target/113282] " pinskia at gcc dot gnu.org
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: patrick at rivosinc dot com @ 2024-01-08 21:25 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113282
Bug ID: 113282
Summary: RISC-V non-atomic union store/load reordering
Product: gcc
Version: 14.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: patrick at rivosinc dot com
Target Milestone: ---
I'm not sure if this is a bug or a valid optimization. Posting this here to
educate myself :)
Testcase:
union {
long c;
int b;
} e;
int f;
int *g = &e.b;
long *h = &e.c;
int main() {
*g = 1;
if (*h == 1)
return 0;
else
return 1;
}
risc-v -O2:
main:
lui a5,%hi(h)
ld a5,%lo(h)(a5)
ld a0,0(a5)
lui a5,%hi(g)
ld a5,%lo(g)(a5) << Load
li a4,1
addi a0,a0,-1
snez a0,a0
sw a4,0(a5) << Store pushed below the load
ret
f:
.zero 4
.zero 4
e:
.zero 8
h:
.dword e
g:
.dword e
My educated guess is that since the operations aren't marked as atomic, the
compiler is free to ignore the dependency and optimize the load before the
store. After marking the ops as atomic:
union {
long c;
int b;
} e;
int f;
int *g = &e.b;
long *h = &e.c;
int main() {
__atomic_store_n(g, 1, __ATOMIC_RELAXED);
if (__atomic_load_n(h, __ATOMIC_RELAXED) == 1)
return 0;
else
return 1;
}
The problem goes away:
main:
lui a5,%hi(g)
ld a5,%lo(g)(a5)
li a4,1
sw a4,0(a5) << Store remains above the load
lui a5,%hi(h)
ld a5,%lo(h)(a5)
ld a0,0(a5) << Load
addi a0,a0,-1
snez a0,a0
ret
f:
.zero 4
.zero 4
e:
.zero 8
h:
.dword e
g:
.dword e
Is that a correct line of reasoning? On x86 this problem doesn't manifest so
it's also possibly a risc-v bug.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113282] RISC-V non-atomic union store/load reordering
2024-01-08 21:25 [Bug target/113282] New: RISC-V non-atomic union store/load reordering patrick at rivosinc dot com
@ 2024-01-08 22:13 ` pinskia at gcc dot gnu.org
2024-01-08 22:15 ` pinskia at gcc dot gnu.org
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-08 22:13 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113282
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |INVALID
Keywords| |alias
Status|UNCONFIRMED |RESOLVED
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Int and long does not alias so moving the store past the load is a valid thing
to do. Note the union does not make a difference here since it is not seen due
taking the address of the fields.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113282] RISC-V non-atomic union store/load reordering
2024-01-08 21:25 [Bug target/113282] New: RISC-V non-atomic union store/load reordering patrick at rivosinc dot com
2024-01-08 22:13 ` [Bug target/113282] " pinskia at gcc dot gnu.org
@ 2024-01-08 22:15 ` pinskia at gcc dot gnu.org
2024-01-08 22:16 ` pinskia at gcc dot gnu.org
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-08 22:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113282
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> On x86 this problem
Yes x86 does not enable the scheduler before ra and has less registers so the
scheduler is not as aggressive as on the other targets.
Note this is standard strict aliasing issue even. Which is definitely mentioned
as a non issue even.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113282] RISC-V non-atomic union store/load reordering
2024-01-08 21:25 [Bug target/113282] New: RISC-V non-atomic union store/load reordering patrick at rivosinc dot com
2024-01-08 22:13 ` [Bug target/113282] " pinskia at gcc dot gnu.org
2024-01-08 22:15 ` pinskia at gcc dot gnu.org
@ 2024-01-08 22:16 ` pinskia at gcc dot gnu.org
2024-01-08 22:40 ` patrick at rivosinc dot com
2024-01-08 22:44 ` pinskia at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-08 22:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113282
--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note -fno-strict-aliasing will allow the code to work the way you want it to
work.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113282] RISC-V non-atomic union store/load reordering
2024-01-08 21:25 [Bug target/113282] New: RISC-V non-atomic union store/load reordering patrick at rivosinc dot com
` (2 preceding siblings ...)
2024-01-08 22:16 ` pinskia at gcc dot gnu.org
@ 2024-01-08 22:40 ` patrick at rivosinc dot com
2024-01-08 22:44 ` pinskia at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: patrick at rivosinc dot com @ 2024-01-08 22:40 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113282
Patrick O'Neill <patrick at rivosinc dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|INVALID |FIXED
--- Comment #4 from Patrick O'Neill <patrick at rivosinc dot com> ---
Thanks for the explanation!
I didn't know about strict aliasing :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113282] RISC-V non-atomic union store/load reordering
2024-01-08 21:25 [Bug target/113282] New: RISC-V non-atomic union store/load reordering patrick at rivosinc dot com
` (3 preceding siblings ...)
2024-01-08 22:40 ` patrick at rivosinc dot com
@ 2024-01-08 22:44 ` pinskia at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-08 22:44 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113282
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|FIXED |INVALID
--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
It is even mentioned here:
https://gcc.gnu.org/bugs/#nonbugs
....
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-08 22:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-08 21:25 [Bug target/113282] New: RISC-V non-atomic union store/load reordering patrick at rivosinc dot com
2024-01-08 22:13 ` [Bug target/113282] " pinskia at gcc dot gnu.org
2024-01-08 22:15 ` pinskia at gcc dot gnu.org
2024-01-08 22:16 ` pinskia at gcc dot gnu.org
2024-01-08 22:40 ` patrick at rivosinc dot com
2024-01-08 22:44 ` pinskia at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).