While testing r12-4376 I noticed that when a fs:0 store is followed by one to gs:0 the former is not emitted, otherwise when each is done on its own each is also emitted on its own. My very basic understanding is that the FS and GS namespaces are distinct with null being a valid address of a distinct location in each (and so I would expect both stores to be emitted) but I leave it experts to confirm or resolve this as invalid. $ cat z.c && gcc -O -S -Wall -o/dev/stdout z.c void test_null_store_fs (void) { int __seg_fs *fs = (int __seg_fs *)0; *fs = 1; // fs:0 store emitted } void test_null_store_gs (void) { int __seg_gs *gs = (int __seg_gs *)0; *gs = 2; // gs:0 store emitted } void test_null_store_fs_gs (void) { int __seg_fs *fs = (int __seg_fs *)0; *fs = 1; // store missing int __seg_gs *gs = (int __seg_gs *)0; *gs = 2; // gs:0 store emitted } .file "z.c" .text .globl test_null_store_fs .type test_null_store_fs, @function test_null_store_fs: .LFB0: .cfi_startproc movl $1, %fs:0 ret .cfi_endproc .LFE0: .size test_null_store_fs, .-test_null_store_fs .globl test_null_store_gs .type test_null_store_gs, @function test_null_store_gs: .LFB1: .cfi_startproc movl $2, %gs:0 ret .cfi_endproc .LFE1: .size test_null_store_gs, .-test_null_store_gs .globl test_null_store_fs_gs .type test_null_store_fs_gs, @function test_null_store_fs_gs: .LFB2: .cfi_startproc movl $2, %gs:0 ret .cfi_endproc .LFE2: .size test_null_store_fs_gs, .-test_null_store_fs_gs .ident "GCC: (GNU) 12.0.0 20211013 (experimental)" .section .note.GNU-stack,"",@progbits
Gimple level is good: MEM[(<address-space-1> int *)0B] = 1; MEM[(<address-space-2> int *)0B] = 2; Expansion seems to be correct (notice AS1 and AS2): (insn 5 4 6 (set (reg/f:DI 82) (const_int 0 [0])) "/app/example.cpp":17:7 -1 (nil)) (insn 6 5 0 (set (mem:SI (reg/f:DI 82) [1 MEM[(<address-space-1> int *)0B]+0 S4 A128 AS1]) (const_int 1 [0x1])) "/app/example.cpp":17:7 -1 (nil)) ;; MEM[(<address-space-2> int *)0B] = 2; (insn 7 6 8 (set (reg/f:DI 83) (const_int 0 [0])) "/app/example.cpp":20:7 -1 (nil)) (insn 8 7 0 (set (mem:SI (reg/f:DI 83) [1 MEM[(<address-space-2> int *)0B]+0 S4 A128 AS2]) (const_int 2 [0x2])) "/app/example.cpp":20:7 -1 (nil)) RTL level DSE removes the first store. I suspect dse.c does not check ADDR_SPACE_GENERIC_P.
(In reply to Andrew Pinski from comment #1) > RTL level DSE removes the first store. I suspect dse.c does not check > ADDR_SPACE_GENERIC_P. I mean MEM_ADDR_SPACE. And yes it looks like it does not check MEM_ADDR_SPACE .... There could be other places in the RTL level which does not check MEM_ADDR_SPACE either.
A couple of data points: I see the same behavior for any constant address (not just null). Clang 12 behaves the same as GCC, but Clang 13 emits both stores, suggesting they fixed it as a bug: https://godbolt.org/z/xn7MWc4qn
(In reply to Martin Sebor from comment #3) > A couple of data points: I see the same behavior for any constant address > (not just null). Right because DSE does not compare MEM_ADDR_SPACE. It is literally a bug in dse.c and there might be other places in the RTL side which misses out on comparing MEM_ADDR_SPACE when it comes to memory accesses because it was an after thought (and was originally added to support SPU memory accesses where a function call would happen).
I think I have a fix.
Created attachment 55238 [details] Patch which I will be testing This adds a check for the address space in DSE. Even tested: ``` void test_null_store_fs_gs (void) { int __seg_fs *fs = (int __seg_fs *)0; *fs = 1; int __seg_gs *gs = (int __seg_gs *)0; *gs = 2; *fs = 3; } ``` And only 2/3 are emitted (in that order) which is the correct DSE even.
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>: https://gcc.gnu.org/g:64ca6aa74b6b5a9737f3808bf4a947dd5c122d47 commit r14-1508-g64ca6aa74b6b5a9737f3808bf4a947dd5c122d47 Author: Andrew Pinski <apinski@marvell.com> Date: Thu Jun 1 21:17:56 2023 -0700 rtl-optimization: [PR102733] DSE removing address which only differ by address space. The problem here is DSE was not taking into account the address space which meant if you had two addresses say `fs:0` and `gs:0` (on x86_64), DSE would think they were the same and remove the first store. This fixes that issue by adding a check for the address space too. OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. PR rtl-optimization/102733 gcc/ChangeLog: * dse.cc (store_info): Add addrspace field. (record_store): Record the address space and check to make sure they are the same. gcc/testsuite/ChangeLog: * gcc.target/i386/addr-space-6.c: New test.
Fixed on the trunk, this was not a regression and has been an issue since __gs/__fs support was added.