[Bug target/102733] New: missing fs:0 store when followed by one to gs:0

[Bug target/102733] New: missing fs:0 store when followed by one to gs:0

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102733

            Bug ID: 102733
           Summary: missing fs:0 store when followed by one to gs:0
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: msebor at gcc dot gnu.org
  Target Milestone: ---

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

[Bug rtl-optimization/102733] missing fs:0 store when followed by one to gs:0

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102733

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
          Component|target                      |rtl-optimization
   Last reconfirmed|                            |2021-10-13
           Keywords|                            |wrong-code

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
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.

[Bug rtl-optimization/102733] missing fs:0 store when followed by one to gs:0

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102733

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(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.

[Bug rtl-optimization/102733] missing fs:0 store when followed by one to gs:0

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102733

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
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

[Bug rtl-optimization/102733] missing fs:0 store when followed by one to gs:0

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102733

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(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).

[Bug rtl-optimization/102733] missing fs:0 store when followed by one to gs:0

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102733

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |pinskia at gcc dot gnu.org

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think I have a fix.

[Bug rtl-optimization/102733] missing fs:0 store when followed by one to gs:0

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102733

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55238
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55238&action=edit
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.

[Bug rtl-optimization/102733] missing fs:0 store when followed by one to gs:0

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102733

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug rtl-optimization/102733] missing fs:0 store when followed by one to gs:0

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102733

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed on the trunk, this was not a regression and has been an issue since
__gs/__fs support was added.