public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/113550] New: data512_t initializers dereference a clobbered register
@ 2024-01-22 23:12 ianthompson at microsoft dot com
2024-01-22 23:26 ` [Bug target/113550] " pinskia at gcc dot gnu.org
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: ianthompson at microsoft dot com @ 2024-01-22 23:12 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113550
Bug ID: 113550
Summary: data512_t initializers dereference a clobbered
register
Product: gcc
Version: 12.2.1
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: ianthompson at microsoft dot com
Target Milestone: ---
Target: aarch64
Created attachment 57189
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57189&action=edit
Additional non-minimal failing cases
When initializing or copying a data512_t, the compiler is generating code which
clobbers the register containing the source pointer of the copy. Initially
observed on Arm GNU Toolchain 12.2.Rel1, but this also reproduces on trunk.
Minimal reproduction, hits a segfault when compiled with "aarch64-none-elf-gcc
-march=armv9-a+ls64":
#include <arm_acle.h>
void test_data512_init() {
data512_t my_value = {};
}
This code generates this assembly snippet for initializing my_value:
adrp x0, .LC0
add x0, x0, :lo12:.LC0
ldp x0, x1, [x0]
ldp x2, x3, [x0, 16]
ldp x4, x5, [x0, 32]
ldp x6, x7, [x0, 48]
Notice that the first ldp clobbers x0, redirecting the remaining 3 loads to
whatever address happens to be in val[0] of the initializer.
Similar incorrect code is generated in many other situations that involve
copying a data512_t (passing a global variable to a function, dereferencing a
data512_t*, etc). See the attached source file for the other failing cases I'm
seeing.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113550] data512_t initializers dereference a clobbered register
2024-01-22 23:12 [Bug target/113550] New: data512_t initializers dereference a clobbered register ianthompson at microsoft dot com
@ 2024-01-22 23:26 ` pinskia at gcc dot gnu.org
2024-01-24 4:31 ` 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-22 23:26 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113550
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2024-01-22
Ever confirmed|0 |1
Keywords| |wrong-code
Status|UNCONFIRMED |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |pinskia at gcc dot gnu.org
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Should be an easy fix.
The pattern:
(define_insn "*aarch64_movv8di"
[(set (match_operand:V8DI 0 "nonimmediate_operand" "=r,m,r")
(match_operand:V8DI 1 "general_operand" " r,r,m"))]
"(register_operand (operands[0], V8DImode)
|| register_operand (operands[1], V8DImode))"
"#"
[(set_attr "type" "multiple,multiple,multiple")
(set_attr "length" "32,16,16")]
)
Is missing a & on the r/m case.
Or the split could be improved such that the one that gets loadded last is the
one that might conflict:
(define_split
[(set (match_operand:V8DI 0 "nonimmediate_operand")
(match_operand:V8DI 1 "general_operand"))]
"reload_completed"
[(const_int 0)]
aarch64_simd_emit_reg_reg_move handles this case already too.
I am going to go with the improving the define_split ...
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113550] data512_t initializers dereference a clobbered register
2024-01-22 23:12 [Bug target/113550] New: data512_t initializers dereference a clobbered register ianthompson at microsoft dot com
2024-01-22 23:26 ` [Bug target/113550] " pinskia at gcc dot gnu.org
@ 2024-01-24 4:31 ` pinskia at gcc dot gnu.org
2024-01-24 7:01 ` 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-24 4:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113550
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Thinking about this, maybe it is too complex to figure out which register
overlaps with the memory. So the easiest is just to mark `=r/m` alternative as
an early clobber.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113550] data512_t initializers dereference a clobbered register
2024-01-22 23:12 [Bug target/113550] New: data512_t initializers dereference a clobbered register ianthompson at microsoft dot com
2024-01-22 23:26 ` [Bug target/113550] " pinskia at gcc dot gnu.org
2024-01-24 4:31 ` pinskia at gcc dot gnu.org
@ 2024-01-24 7:01 ` pinskia at gcc dot gnu.org
2024-01-25 12:03 ` cvs-commit at gcc dot gnu.org
2024-01-25 12:14 ` rsandifo at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-24 7:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113550
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
URL| |https://gcc.gnu.org/piperma
| |il/gcc-patches/2024-January
| |/643766.html
Keywords| |patch
--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Patch posted:
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/643766.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113550] data512_t initializers dereference a clobbered register
2024-01-22 23:12 [Bug target/113550] New: data512_t initializers dereference a clobbered register ianthompson at microsoft dot com
` (2 preceding siblings ...)
2024-01-24 7:01 ` pinskia at gcc dot gnu.org
@ 2024-01-25 12:03 ` cvs-commit at gcc dot gnu.org
2024-01-25 12:14 ` rsandifo at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-01-25 12:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113550
--- Comment #4 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Richard Sandiford <rsandifo@gcc.gnu.org>:
https://gcc.gnu.org/g:8eead1148cd0ac086b39a7abccea404041c85cb5
commit r14-8421-g8eead1148cd0ac086b39a7abccea404041c85cb5
Author: Richard Sandiford <richard.sandiford@arm.com>
Date: Thu Jan 25 12:03:18 2024 +0000
aarch64: Handle overlapping registers in movv8di [PR113550]
The LS64 movv8di pattern didn't handle loads that overlapped with
the address register (unless the overlap happened to be in the
last subload).
gcc/
PR target/113550
* config/aarch64/aarch64-simd.md: In the movv8di splitter, check
whether each split instruction is a load that clobbers the source
address. Emit that instruction last if so.
gcc/testsuite/
PR target/113550
* gcc.target/aarch64/pr113550.c: New test.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug target/113550] data512_t initializers dereference a clobbered register
2024-01-22 23:12 [Bug target/113550] New: data512_t initializers dereference a clobbered register ianthompson at microsoft dot com
` (3 preceding siblings ...)
2024-01-25 12:03 ` cvs-commit at gcc dot gnu.org
@ 2024-01-25 12:14 ` rsandifo at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: rsandifo at gcc dot gnu.org @ 2024-01-25 12:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113550
Richard Sandiford <rsandifo at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rsandifo at gcc dot gnu.org
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #5 from Richard Sandiford <rsandifo at gcc dot gnu.org> ---
Yeah, FWIW, I agree improving the define_split is probably best.
Now fixed.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-25 12:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-22 23:12 [Bug target/113550] New: data512_t initializers dereference a clobbered register ianthompson at microsoft dot com
2024-01-22 23:26 ` [Bug target/113550] " pinskia at gcc dot gnu.org
2024-01-24 4:31 ` pinskia at gcc dot gnu.org
2024-01-24 7:01 ` pinskia at gcc dot gnu.org
2024-01-25 12:03 ` cvs-commit at gcc dot gnu.org
2024-01-25 12:14 ` rsandifo 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).