public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c/111613] New: Bit field stores can be incorrectly optimized away when -fstore-merging is in effect
@ 2023-09-27 12:51 gcc at kempniu dot pl
2023-09-27 13:52 ` [Bug ipa/111613] [12/13/14 Regression] " rguenth at gcc dot gnu.org
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: gcc at kempniu dot pl @ 2023-09-27 12:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111613
Bug ID: 111613
Summary: Bit field stores can be incorrectly optimized away
when -fstore-merging is in effect
Product: gcc
Version: 12.1.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c
Assignee: unassigned at gcc dot gnu.org
Reporter: gcc at kempniu dot pl
Target Milestone: ---
Created attachment 56002
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56002&action=edit
Preprocessed reproducer
This is the simplest reproducer I could come up with:
$ cat bitfield.c
#include <stdio.h>
#include <stdlib.h>
struct bitfield {
unsigned int field1 : 1;
unsigned int field2 : 1;
unsigned int field3 : 1;
};
__attribute__((noinline)) static void
set_field1_and_field2(struct bitfield *b) {
b->field1 = 1;
b->field2 = 1;
}
__attribute__((noinline)) static struct bitfield *
new_bitfield(void) {
struct bitfield *b = (struct bitfield *)malloc(sizeof(*b));
b->field3 = 1;
set_field1_and_field2(b);
return b;
}
int main(void) {
struct bitfield *b = new_bitfield();
printf("%d\n", b->field3);
return 0;
}
$ gcc -O2 -o bitfield bitfield.c
$ ./bitfield
0
$ gcc -O2 -fno-store-merging -o bitfield bitfield.c
$ ./bitfield
1
Removing one of the stores from set_field1_and_field2() makes the issue
go away. Moving "b->field3 = 1;" below the call to
set_field1_and_field2() also makes the issue go away.
This was originally found for the following GCC version:
$ gcc -v
Using built-in specs.
COLLECT_GCC=/usr/bin/gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/13.2.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure
--enable-languages=ada,c,c++,d,fortran,go,lto,objc,obj-c++ --enable-bootstrap
--prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man
--infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/
--with-build-config=bootstrap-lto --with-linker-hash-style=gnu
--with-system-zlib --enable-__cxa_atexit --enable-cet=auto
--enable-checking=release --enable-clocale=gnu --enable-default-pie
--enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object
--enable-libstdcxx-backtrace --enable-link-serialization=1
--enable-linker-build-id --enable-lto --enable-multilib --enable-plugin
--enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch
--disable-werror
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.2.1 20230801 (GCC)
However, I have subsequently confirmed that it also happens on the
current master branch.
A git bisect points at the following commit as the culprit (first
included in GCC 12.1.0):
commit 22c242342e38ebffa6bbf7e86e7a1e4abdf0d686
Author: Martin Liska <mliska@suse.cz>
Date: Thu Nov 18 17:50:19 2021 +0100
IPA: fix reproducibility in IPA MOD REF
gcc/ChangeLog:
* ipa-modref.c (analyze_function): Do not execute the code
only if dump_file != NULL.
Reverting this change on top of current master makes the issue
disappear, so it looks legit to me.
Disassembly of new_bitfield() for "gcc -O2":
0x0000000000001190 <+0>: sub $0x8,%rsp
0x0000000000001194 <+4>: mov $0x4,%edi
0x0000000000001199 <+9>: call 0x1040 <malloc@plt>
0x000000000000119e <+14>: mov %rax,%rdi
0x00000000000011a1 <+17>: call 0x1180 <set_field1_and_field2>
0x00000000000011a6 <+22>: add $0x8,%rsp
0x00000000000011aa <+26>: ret
Disassembly of new_bitfield() for "gcc -O2 -fno-store-merging":
0x0000000000001190 <+0>: sub $0x8,%rsp
0x0000000000001194 <+4>: mov $0x4,%edi
0x0000000000001199 <+9>: call 0x1040 <malloc@plt>
0x000000000000119e <+14>: orb $0x4,(%rax)
0x00000000000011a1 <+17>: mov %rax,%rdi
0x00000000000011a4 <+20>: call 0x1180 <set_field1_and_field2>
0x00000000000011a9 <+25>: add $0x8,%rsp
0x00000000000011ad <+29>: ret
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug ipa/111613] [12/13/14 Regression] Bit field stores can be incorrectly optimized away when -fstore-merging is in effect
2023-09-27 12:51 [Bug c/111613] New: Bit field stores can be incorrectly optimized away when -fstore-merging is in effect gcc at kempniu dot pl
@ 2023-09-27 13:52 ` rguenth at gcc dot gnu.org
2023-09-27 13:53 ` rguenth at gcc dot gnu.org
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-09-27 13:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111613
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Keywords| |wrong-code
Target Milestone|--- |12.4
Ever confirmed|0 |1
Priority|P3 |P1
Summary|Bit field stores can be |[12/13/14 Regression] Bit
|incorrectly optimized away |field stores can be
|when -fstore-merging is in |incorrectly optimized away
|effect |when -fstore-merging is in
| |effect
Last reconfirmed| |2023-09-27
Component|c |ipa
CC| |hubicka at gcc dot gnu.org,
| |jamborm at gcc dot gnu.org,
| |marxin at gcc dot gnu.org,
| |rguenth at gcc dot gnu.org
--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug ipa/111613] [12/13/14 Regression] Bit field stores can be incorrectly optimized away when -fstore-merging is in effect
2023-09-27 12:51 [Bug c/111613] New: Bit field stores can be incorrectly optimized away when -fstore-merging is in effect gcc at kempniu dot pl
2023-09-27 13:52 ` [Bug ipa/111613] [12/13/14 Regression] " rguenth at gcc dot gnu.org
@ 2023-09-27 13:53 ` rguenth at gcc dot gnu.org
2023-10-30 13:24 ` pinskia at gcc dot gnu.org
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-09-27 13:53 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111613
--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
It's the late IPA modref that mis-analyzes the store-merged sequence I think.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug ipa/111613] [12/13/14 Regression] Bit field stores can be incorrectly optimized away when -fstore-merging is in effect
2023-09-27 12:51 [Bug c/111613] New: Bit field stores can be incorrectly optimized away when -fstore-merging is in effect gcc at kempniu dot pl
2023-09-27 13:52 ` [Bug ipa/111613] [12/13/14 Regression] " rguenth at gcc dot gnu.org
2023-09-27 13:53 ` rguenth at gcc dot gnu.org
@ 2023-10-30 13:24 ` pinskia at gcc dot gnu.org
2023-11-07 9:23 ` rguenth at gcc dot gnu.org
2023-12-15 12:50 ` rguenth at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-10-30 13:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111613
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |Sean.XiaoXiang at outlook dot com
--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 112289 has been marked as a duplicate of this bug. ***
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug ipa/111613] [12/13/14 Regression] Bit field stores can be incorrectly optimized away when -fstore-merging is in effect
2023-09-27 12:51 [Bug c/111613] New: Bit field stores can be incorrectly optimized away when -fstore-merging is in effect gcc at kempniu dot pl
` (2 preceding siblings ...)
2023-10-30 13:24 ` pinskia at gcc dot gnu.org
@ 2023-11-07 9:23 ` rguenth at gcc dot gnu.org
2023-12-15 12:50 ` rguenth at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-11-07 9:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111613
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed|2023-09-27 00:00:00 |2023-11-7
--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
DSE removes the store before set_field1_and_field2:
b_3 = malloc (4);
- b_3->field3 = 1;
set_field1_and_field2 (b_3);
modref analyzing 'set_field1_and_field2/22' (ipa=0)
Past summary:
loads:
stores:
Base 0: alias set 1
Ref 0: alias set 1
access: Parm 0 param offset:0 offset:0 size:1 max_size:2
kills:
Parm 0 param offset:0 offset:0 size:2 max_size:2
Try dse
parm 0 flags: no_indirect_clobber no_direct_escape no_indirect_escape
no_direct_read no_indirect_read
and local analysis:
- Analyzing load: MEM <unsigned char> [(struct bitfield *)b_2(D)]
- Recording base_set=1 ref_set=1 Parm 0 param offset:0 offset:0 size:8
max_size:8
- Analyzing store: MEM <unsigned char> [(struct bitfield *)b_2(D)]
- Recording base_set=1 ref_set=1 Parm 0 param offset:0 offset:0 size:8
max_size:8
- Recording kill
- modref done with result: tracked.
loads:
Base 0: alias set 1
Ref 0: alias set 1
access: Parm 0 param offset:0 offset:0 size:8 max_size:8
stores:
Base 0: alias set 1
Ref 0: alias set 1
access: Parm 0 param offset:0 offset:0 size:8 max_size:8
kills:
Parm 0 param offset:0 offset:0 size:8 max_size:8
Try dse
parm 0 flags: no_indirect_clobber no_direct_escape no_indirect_escape
no_direct_read no_indirect_read
note how we merge flags of the param to include the IPA time no_direct_read.
Basically the store-merging transform changes the store to kill all bits
but by reading the original bits and storing them again.
The change by Martin made us use the IPA param flags, but as shown we
really can't do this, at least not for all flag kinds. So I don't think
we can use the IPA state for the function we're re-analyzing, but we
can probably use the IPA of function calls we run into during analyzing?
Honza, please have a look here.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug ipa/111613] [12/13/14 Regression] Bit field stores can be incorrectly optimized away when -fstore-merging is in effect
2023-09-27 12:51 [Bug c/111613] New: Bit field stores can be incorrectly optimized away when -fstore-merging is in effect gcc at kempniu dot pl
` (3 preceding siblings ...)
2023-11-07 9:23 ` rguenth at gcc dot gnu.org
@ 2023-12-15 12:50 ` rguenth at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-12-15 12:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111613
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Priority|P1 |P2
Assignee|unassigned at gcc dot gnu.org |hubicka at gcc dot gnu.org
Status|NEW |ASSIGNED
--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Yours.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-12-15 12:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-27 12:51 [Bug c/111613] New: Bit field stores can be incorrectly optimized away when -fstore-merging is in effect gcc at kempniu dot pl
2023-09-27 13:52 ` [Bug ipa/111613] [12/13/14 Regression] " rguenth at gcc dot gnu.org
2023-09-27 13:53 ` rguenth at gcc dot gnu.org
2023-10-30 13:24 ` pinskia at gcc dot gnu.org
2023-11-07 9:23 ` rguenth at gcc dot gnu.org
2023-12-15 12:50 ` rguenth 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).