* [Bug translation/96159] atomic creates incorrect code for possible isaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
@ 2020-07-10 20:34 ` uecker at eecs dot berkeley.edu
2020-07-10 20:37 ` uecker at eecs dot berkeley.edu
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: uecker at eecs dot berkeley.edu @ 2020-07-10 20:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
--- Comment #1 from Martin Uecker <uecker at eecs dot berkeley.edu> ---
On x86-64 the following struct has alignment 4 but gcc
creates a single mov instruction which according to my
understanding may fail to be atomic when it crosses a
cache line boundary.
Documentation seems to imply that __atomic_load is
supposed to work for all types. It also compiles
without warning.
typedef struct { float re; float im; } foo_t;
int foo_align(void)
{
foo_t x;
return _Alignof(x);
}
foo_t foo_load(foo_t* x)
{
foo_t r;
__atomic_load(x, &r, __ATOMIC_SEQ_CST);
return r;
}
assembler:
foo_align:
mov eax, 4
ret
foo_load:
movq xmm0, QWORD PTR [rdi]
ret
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug translation/96159] atomic creates incorrect code for possible isaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
2020-07-10 20:34 ` [Bug translation/96159] " uecker at eecs dot berkeley.edu
@ 2020-07-10 20:37 ` uecker at eecs dot berkeley.edu
2021-11-07 10:51 ` [Bug middle-end/96159] " muecker at gwdg dot de
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: uecker at eecs dot berkeley.edu @ 2020-07-10 20:37 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
--- Comment #2 from Martin Uecker <uecker at eecs dot berkeley.edu> ---
Clang produces a call to __atomic_load.
Also here is a godbolt link: https://godbolt.org/z/39PE1G
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug middle-end/96159] atomic creates incorrect code for possible isaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
2020-07-10 20:34 ` [Bug translation/96159] " uecker at eecs dot berkeley.edu
2020-07-10 20:37 ` uecker at eecs dot berkeley.edu
@ 2021-11-07 10:51 ` muecker at gwdg dot de
2021-11-07 20:29 ` pinskia at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: muecker at gwdg dot de @ 2021-11-07 10:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
--- Comment #3 from Martin Uecker <muecker at gwdg dot de> ---
The documentation:
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html#g_t_005f_005fatomic-Builtins
"The four non-arithmetic functions (load, store, exchange, and
compare_exchange) all have a generic version as well. This generic version
works on any data type.
It uses the lock-free built-in function if the specific data type size makes
that possible; otherwise, an external call is left to be resolved at run time."
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug middle-end/96159] atomic creates incorrect code for possible isaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
` (2 preceding siblings ...)
2021-11-07 10:51 ` [Bug middle-end/96159] " muecker at gwdg dot de
@ 2021-11-07 20:29 ` pinskia at gcc dot gnu.org
2021-11-26 9:07 ` redi at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-11-07 20:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So it looks like __atomic_load assumes the type is _Atomic which has the right
alignment to it.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug middle-end/96159] atomic creates incorrect code for possible isaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
` (3 preceding siblings ...)
2021-11-07 20:29 ` pinskia at gcc dot gnu.org
@ 2021-11-26 9:07 ` redi at gcc dot gnu.org
2021-11-26 9:11 ` redi at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2021-11-26 9:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords|ABI, wrong-code |documentation
Ever confirmed|0 |1
Last reconfirmed| |2021-11-26
Status|UNCONFIRMED |NEW
--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It assumes that because the purpose of those built-ins is to implement C
_Atomic and C++ std::atomic. If you use those APIs the alignment is taken care
of n by the implementation. If you use the low-level, non-portable APIs
directly then you are responsible for getting it right.
This is just a documentation bug.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug middle-end/96159] atomic creates incorrect code for possible isaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
` (4 preceding siblings ...)
2021-11-26 9:07 ` redi at gcc dot gnu.org
@ 2021-11-26 9:11 ` redi at gcc dot gnu.org
2021-11-26 15:29 ` muecker at gwdg dot de
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: redi at gcc dot gnu.org @ 2021-11-26 9:11 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Martin Uecker from comment #3)
> "The four non-arithmetic functions (load, store, exchange, and
> compare_exchange) all have a generic version as well. This generic version
> works on any data type.
This is true. They work on your type, but not for all values of that type, only
ones that are appropriately aligned.
> It uses the lock-free built-in function if the specific data type size makes
> that possible;
As it says, it only depends on the size, there is no check for alignment.
What's missing is documentation about the requirement for the user to ensure
the right alignment.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug middle-end/96159] atomic creates incorrect code for possible isaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
` (5 preceding siblings ...)
2021-11-26 9:11 ` redi at gcc dot gnu.org
@ 2021-11-26 15:29 ` muecker at gwdg dot de
2021-11-26 17:02 ` gabravier at gmail dot com
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: muecker at gwdg dot de @ 2021-11-26 15:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
--- Comment #7 from Martin Uecker <muecker at gwdg dot de> ---
I do not think these functions are meant only as internal tools to implement
the language features.
We also seem to agree that the documentation implies that there should work for
all types and does not currently mention alignment.
One can of course now retrospectively change the documentation to fix this bug.
I would rather prefer to make this work as it is very useful functionality.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug middle-end/96159] atomic creates incorrect code for possible isaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
` (6 preceding siblings ...)
2021-11-26 15:29 ` muecker at gwdg dot de
@ 2021-11-26 17:02 ` gabravier at gmail dot com
2021-11-26 18:19 ` muecker at gwdg dot de
2021-11-29 6:22 ` [Bug middle-end/96159] atomic creates incorrect code for possible misaligned struct muecker at gwdg dot de
9 siblings, 0 replies; 11+ messages in thread
From: gabravier at gmail dot com @ 2021-11-26 17:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
Gabriel Ravier <gabravier at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |gabravier at gmail dot com
--- Comment #8 from Gabriel Ravier <gabravier at gmail dot com> ---
I do agree that either:
- GCC's behavior should be aligned with Clang's and that it should provide some
kind of "known-aligned load" function (along with the corresponding ones for
store/exchange/compare_exchange)
- or there should at least be some kind of "safe load" function (along with the
corresponding ones) for the cases where the alignment is unknown.
I personally do prefer the first solution, personally, unless the Clang devs
can be convinced to change their builtin, as I think it would be better to have
the same behavior on both compilers.
In any case, though, I do think there is a documentation bug here, and I would
also say it would be quite nice to have a warning when using the built-in in a
way that makes it much slower/invalid, like what Clang does:
<source>:240:5: warning: misaligned atomic operation may incur significant
performance penalty; the expected alignment (8 bytes) exceeds the actual
alignment (4 bytes) [-Watomic-alignment]
__atomic_load(x, &r, __ATOMIC_SEQ_CST);
^
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug middle-end/96159] atomic creates incorrect code for possible isaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
` (7 preceding siblings ...)
2021-11-26 17:02 ` gabravier at gmail dot com
@ 2021-11-26 18:19 ` muecker at gwdg dot de
2021-11-29 6:22 ` [Bug middle-end/96159] atomic creates incorrect code for possible misaligned struct muecker at gwdg dot de
9 siblings, 0 replies; 11+ messages in thread
From: muecker at gwdg dot de @ 2021-11-26 18:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
--- Comment #9 from Martin Uecker <muecker at gwdg dot de> ---
Yes the clang behavior is useful.
If we get the optimal code for types with sufficient alignment, then I do not
think a separate set of functions would be required. A programmer simply can
use the _Atomic(T) types which have optimal alignment to get fastest code
possible and somebody who needs these functions on existing data structures
could get the safe access by default (and maybe a warning it alignment is not
optimal). This would appear to be the ideal design.
Even if we document the alignment requirement, it is still very dangerous to
use because alignment requirement may not be identical for all architectures.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug middle-end/96159] atomic creates incorrect code for possible misaligned struct
2020-07-10 20:28 [Bug translation/96159] New: atomic creates incorrect code for possible isaligned struct uecker at eecs dot berkeley.edu
` (8 preceding siblings ...)
2021-11-26 18:19 ` muecker at gwdg dot de
@ 2021-11-29 6:22 ` muecker at gwdg dot de
9 siblings, 0 replies; 11+ messages in thread
From: muecker at gwdg dot de @ 2021-11-29 6:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96159
Martin Uecker <muecker at gwdg dot de> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|atomic creates incorrect |atomic creates incorrect
|code for possible isaligned |code for possible
|struct |misaligned struct
--- Comment #10 from Martin Uecker <muecker at gwdg dot de> ---
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87237
^ permalink raw reply [flat|nested] 11+ messages in thread