public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address
@ 2023-06-02 22:18 ianthompson at microsoft dot com
2023-06-02 22:28 ` [Bug target/110100] " pinskia at gcc dot gnu.org
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: ianthompson at microsoft dot com @ 2023-06-02 22:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
Bug ID: 110100
Summary: __builtin_aarch64_st64b stores to the wrong address
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: ---
Created attachment 55247
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55247&action=edit
Preprocessed reproduction, compile with "aarch64-none-elf-gcc -march=armv8.7-a
-c"
On aarch64, the __arm_st64b function (which is just a thin wrapper around
__builtin_aarch64_st64b) produces incorrect code which uses an uninitialized
register as the address for an ST64B store instruction.
A full preprocessed file is attached, but this snippet is sufficient to
reproduce:
#include <arm_acle.h>
void do_st64b(data512_t data) {
__arm_st64b((void*)0x10000000, data);
}
Compiling with "aarch64-none-elf-gcc -O2 -march=armv8.7-a", I get the following
assembly snippet:
do_st64b:
ldp x8, x9, [x0]
ldp x10, x11, [x0, 16]
ldp x12, x13, [x0, 32]
ldp x14, x15, [x0, 48]
st64b x8, [x1]
ret
Notice that the st64b instruction uses the uninitialized register x1 as an
address, and the constant 0x10000000 is not loaded. I turned optimizations on
to keep the assembly small, but the same issue also occurs without
optimizations.
Digging a bit into aarch64-builtins.cc, it seems like
AARCH64_LS64_BUILTIN_ST64B incorrectly declares the address register an output
instead of an input, leading to the uninitialized register seen above.
The builtins for LD64B, ST64BV, and ST64BV0 appear to be fine, it's just ST64B
which has this issue.
Initially discovered on Arm's build of the toolchain, but I can also reproduce
on trunk and an official 13.1.0 release:
aarch64-none-elf-gcc (Arm GNU Toolchain 12.2.Rel1 (Build arm-12.24)) 12.2.1
20221205
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
@ 2023-06-02 22:28 ` pinskia at gcc dot gnu.org
2023-06-06 9:43 ` acoplan at gcc dot gnu.org
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-06-02 22:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Keywords| |wrong-code
Ever confirmed|0 |1
Last reconfirmed| |2023-06-02
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed, you can get the similar wrong code with just:
#include <arm_acle.h>
void do_st64b(int *a, data512_t data) {
__arm_st64b((void*)(a+512), data);
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
2023-06-02 22:28 ` [Bug target/110100] " pinskia at gcc dot gnu.org
@ 2023-06-06 9:43 ` acoplan at gcc dot gnu.org
2023-06-07 16:45 ` cvs-commit at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: acoplan at gcc dot gnu.org @ 2023-06-06 9:43 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
Alex Coplan <acoplan at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
CC| |acoplan at gcc dot gnu.org
Assignee|unassigned at gcc dot gnu.org |acoplan at gcc dot gnu.org
--- Comment #2 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Testing a fix.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
2023-06-02 22:28 ` [Bug target/110100] " pinskia at gcc dot gnu.org
2023-06-06 9:43 ` acoplan at gcc dot gnu.org
@ 2023-06-07 16:45 ` cvs-commit at gcc dot gnu.org
2023-06-07 16:45 ` cvs-commit at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-07 16:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alex Coplan <acoplan@gcc.gnu.org>:
https://gcc.gnu.org/g:713613541254039a34e1dd8fd4a613a299af1fd6
commit r14-1615-g713613541254039a34e1dd8fd4a613a299af1fd6
Author: Alex Coplan <alex.coplan@arm.com>
Date: Tue Jun 6 11:04:45 2023 +0100
aarch64: Fix whitespace in ls64 builtin implementation [PR110100]
The ls64 builtin code was using incorrect GNU style with eight spaces where
there should be a tab. Fixed thusly.
gcc/ChangeLog:
PR target/110100
* config/aarch64/aarch64-builtins.cc
(aarch64_init_ls64_builtins_types):
Replace eight consecutive spaces with tabs.
(aarch64_init_ls64_builtins): Likewise.
(aarch64_expand_builtin_ls64): Likewise.
* config/aarch64/aarch64.md (ld64b): Likewise.
(st64b): Likewise.
(st64bv): Likewise
(st64bv0): Likewise.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
` (2 preceding siblings ...)
2023-06-07 16:45 ` cvs-commit at gcc dot gnu.org
@ 2023-06-07 16:45 ` cvs-commit at gcc dot gnu.org
2023-06-07 16:48 ` acoplan at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-07 16:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alex Coplan <acoplan@gcc.gnu.org>:
https://gcc.gnu.org/g:737a0b749a7bc3e7cb904ea2d4b18dc130514b85
commit r14-1616-g737a0b749a7bc3e7cb904ea2d4b18dc130514b85
Author: Alex Coplan <alex.coplan@arm.com>
Date: Tue Jun 6 11:52:19 2023 +0100
aarch64: Fix wrong code with st64b builtin [PR110100]
The st64b pattern incorrectly had an output constraint on the register
operand containing the destination address for the store, leading to
wrong code. This patch fixes that.
gcc/ChangeLog:
PR target/110100
* config/aarch64/aarch64-builtins.cc (aarch64_expand_builtin_ls64):
Use input operand for the destination address.
* config/aarch64/aarch64.md (st64b): Fix constraint on address
operand.
gcc/testsuite/ChangeLog:
PR target/110100
* gcc.target/aarch64/acle/pr110100.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
` (3 preceding siblings ...)
2023-06-07 16:45 ` cvs-commit at gcc dot gnu.org
@ 2023-06-07 16:48 ` acoplan at gcc dot gnu.org
2023-06-20 21:22 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: acoplan at gcc dot gnu.org @ 2023-06-07 16:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
--- Comment #5 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Fixed on trunk for GCC 14, keeping open for backports (I think we need this
back to GCC 12).
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
` (4 preceding siblings ...)
2023-06-07 16:48 ` acoplan at gcc dot gnu.org
@ 2023-06-20 21:22 ` cvs-commit at gcc dot gnu.org
2023-06-20 21:22 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-20 21:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:
https://gcc.gnu.org/g:ff00fa1914e42d6b9c45cb36a5c99f94c4133cba
commit r13-7460-gff00fa1914e42d6b9c45cb36a5c99f94c4133cba
Author: Alex Coplan <alex.coplan@arm.com>
Date: Tue Jun 6 11:04:45 2023 +0100
aarch64: Fix whitespace in ls64 builtin implementation [PR110100]
The ls64 builtin code was using incorrect GNU style with eight spaces where
there should be a tab. Fixed thusly.
gcc/ChangeLog:
PR target/110100
* config/aarch64/aarch64-builtins.cc
(aarch64_init_ls64_builtins_types):
Replace eight consecutive spaces with tabs.
(aarch64_init_ls64_builtins): Likewise.
(aarch64_expand_builtin_ls64): Likewise.
* config/aarch64/aarch64.md (ld64b): Likewise.
(st64b): Likewise.
(st64bv): Likewise
(st64bv0): Likewise.
(cherry picked from commit 713613541254039a34e1dd8fd4a613a299af1fd6)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
` (5 preceding siblings ...)
2023-06-20 21:22 ` cvs-commit at gcc dot gnu.org
@ 2023-06-20 21:22 ` cvs-commit at gcc dot gnu.org
2023-06-22 10:19 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-20 21:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:
https://gcc.gnu.org/g:9df688cbf908adc43e92bd012dafa88680ea11dc
commit r13-7461-g9df688cbf908adc43e92bd012dafa88680ea11dc
Author: Alex Coplan <alex.coplan@arm.com>
Date: Tue Jun 6 11:52:19 2023 +0100
aarch64: Fix wrong code with st64b builtin [PR110100]
The st64b pattern incorrectly had an output constraint on the register
operand containing the destination address for the store, leading to
wrong code. This patch fixes that.
gcc/ChangeLog:
PR target/110100
* config/aarch64/aarch64-builtins.cc (aarch64_expand_builtin_ls64):
Use input operand for the destination address.
* config/aarch64/aarch64.md (st64b): Fix constraint on address
operand.
gcc/testsuite/ChangeLog:
PR target/110100
* gcc.target/aarch64/acle/pr110100.c: New test.
(cherry picked from commit 737a0b749a7bc3e7cb904ea2d4b18dc130514b85)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
` (6 preceding siblings ...)
2023-06-20 21:22 ` cvs-commit at gcc dot gnu.org
@ 2023-06-22 10:19 ` cvs-commit at gcc dot gnu.org
2023-06-22 10:19 ` cvs-commit at gcc dot gnu.org
2023-06-22 10:20 ` acoplan at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-22 10:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:
https://gcc.gnu.org/g:c0ab0d4af51382d9d8d4e6b026865842d8e06d7e
commit r12-9718-gc0ab0d4af51382d9d8d4e6b026865842d8e06d7e
Author: Alex Coplan <alex.coplan@arm.com>
Date: Tue Jun 6 11:04:45 2023 +0100
aarch64: Fix whitespace in ls64 builtin implementation [PR110100]
The ls64 builtin code was using incorrect GNU style with eight spaces where
there should be a tab. Fixed thusly.
gcc/ChangeLog:
PR target/110100
* config/aarch64/aarch64-builtins.cc
(aarch64_init_ls64_builtins_types):
Replace eight consecutive spaces with tabs.
(aarch64_init_ls64_builtins): Likewise.
(aarch64_expand_builtin_ls64): Likewise.
* config/aarch64/aarch64.md (ld64b): Likewise.
(st64b): Likewise.
(st64bv): Likewise
(st64bv0): Likewise.
(cherry picked from commit 713613541254039a34e1dd8fd4a613a299af1fd6)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
` (7 preceding siblings ...)
2023-06-22 10:19 ` cvs-commit at gcc dot gnu.org
@ 2023-06-22 10:19 ` cvs-commit at gcc dot gnu.org
2023-06-22 10:20 ` acoplan at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-06-22 10:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Alex Coplan
<acoplan@gcc.gnu.org>:
https://gcc.gnu.org/g:0112ed013847ca9dbef4ba21f1c3f94c5bbe310b
commit r12-9719-g0112ed013847ca9dbef4ba21f1c3f94c5bbe310b
Author: Alex Coplan <alex.coplan@arm.com>
Date: Tue Jun 6 11:52:19 2023 +0100
aarch64: Fix wrong code with st64b builtin [PR110100]
The st64b pattern incorrectly had an output constraint on the register
operand containing the destination address for the store, leading to
wrong code. This patch fixes that.
gcc/ChangeLog:
PR target/110100
* config/aarch64/aarch64-builtins.cc (aarch64_expand_builtin_ls64):
Use input operand for the destination address.
* config/aarch64/aarch64.md (st64b): Fix constraint on address
operand.
gcc/testsuite/ChangeLog:
PR target/110100
* gcc.target/aarch64/acle/pr110100.c: New test.
(cherry picked from commit 737a0b749a7bc3e7cb904ea2d4b18dc130514b85)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug target/110100] __builtin_aarch64_st64b stores to the wrong address
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
` (8 preceding siblings ...)
2023-06-22 10:19 ` cvs-commit at gcc dot gnu.org
@ 2023-06-22 10:20 ` acoplan at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: acoplan at gcc dot gnu.org @ 2023-06-22 10:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110100
Alex Coplan <acoplan at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #10 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Should be fixed on all affected branches, thanks for the report.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-06-22 10:20 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-02 22:18 [Bug target/110100] New: __builtin_aarch64_st64b stores to the wrong address ianthompson at microsoft dot com
2023-06-02 22:28 ` [Bug target/110100] " pinskia at gcc dot gnu.org
2023-06-06 9:43 ` acoplan at gcc dot gnu.org
2023-06-07 16:45 ` cvs-commit at gcc dot gnu.org
2023-06-07 16:45 ` cvs-commit at gcc dot gnu.org
2023-06-07 16:48 ` acoplan at gcc dot gnu.org
2023-06-20 21:22 ` cvs-commit at gcc dot gnu.org
2023-06-20 21:22 ` cvs-commit at gcc dot gnu.org
2023-06-22 10:19 ` cvs-commit at gcc dot gnu.org
2023-06-22 10:19 ` cvs-commit at gcc dot gnu.org
2023-06-22 10:20 ` acoplan 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).