Ping. https://gcc.gnu.org/pipermail/gcc-patches/2026-October/733820.html Thanks, Kyrill
> On 8 Oct 2026, at 11:12, Kyrylo Tkachov <[email protected]> wrote: > > From: Kyrylo Tkachov <[email protected]> > > When cselib records a store of a value V to (mem:M A), add_mem_for_addr > adds V to the addr_list of the value of A, and it adds the MEM to the > locations of V. An addr_list entry is only a value: it does not record > the address space of the MEM. The address space is only in the MEM > locations of V, and V can also have MEM locations at other addresses. > > To look up (mem:M A) in address space AS, cselib_lookup_mem walks the > addr_list of A. It accepts a value of mode M if any MEM location of > that value is in AS, even a MEM at another address. The MEM at A, which > put the value on the list, can be in another address space. For > example, on x86: > > static const long tab[4] = { 1, 2, 3, 4 }; > > long > f (int i, long __seg_gs *p) > { > long *q = (long *) (__UINTPTR_TYPE__) p; > *p = tab[i]; > return *q; > } > > The value of TAB[I] has a generic MEM location in TAB. The %gs store > does not invalidate it, because TAB is read-only. The %gs store adds > the value to the addr_list of P. The generic load from Q, whose address > has the same value as P, finds the value on that list and accepts it > because of its location in TAB. postreload then replaces the load with > a register. With -O2 -fno-dse (without -fno-dse, a similar bug in DSE > removes the load earlier): > > movslq %edi, %rdi > movq tab(,%rdi,8), %rax > movq %rax, %gs:(%rsi) > ret > > Check the address space of the MEM location at A. This is the same test > that add_mem_for_addr uses to avoid duplicates: the same address value > and the same address space. Also say in the comment of addr_list that > its MEMs can be in any address space. > > gcc/ChangeLog: > > * cselib.h (struct cselib_val): Say in the comment of addr_list > that its MEMs can be in any address space. > * cselib.cc (cselib_has_mem_loc_p): New function. > (cselib_lookup_mem): Use it to check the address space of the > memory location at the address of X. > > gcc/testsuite/ChangeLog: > > * gcc.target/i386/addr-space-9.c: New test. > > Signed-off-by: Kyrylo Tkachov <[email protected]> > --- > Changes since v2: > - Describe the data structures and the problem more clearly (Jeff, > Eric). > - Say in the comment of addr_list that its MEMs can be in any address > space. No code change. > > gcc/cselib.cc | 30 ++++++++++++++------ > gcc/cselib.h | 3 +- > gcc/testsuite/gcc.target/i386/addr-space-9.c | 19 +++++++++++++ > 3 files changed, 43 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/addr-space-9.c > > diff --git a/gcc/cselib.cc b/gcc/cselib.cc > index 3e48fe5dc..5567caa47 100644 > --- a/gcc/cselib.cc > +++ b/gcc/cselib.cc > @@ -1765,6 +1765,24 @@ add_mem_for_addr (cselib_val *addr_elt, cselib_val > *mem_elt, rtx x) > } > } > > +/* Return true if V has a memory location at address ADDR in address > + space AS. */ > + > +static bool > +cselib_has_mem_loc_p (cselib_val *v, cselib_val *addr, addr_space_t as) > +{ > + for (elt_loc_list *l = v->locs; l; l = l->next) > + if (MEM_P (l->loc) && MEM_ADDR_SPACE (l->loc) == as) > + { > + /* add_mem_for_addr gives each memory location a VALUE address. */ > + rtx loc_addr = XEXP (l->loc, 0); > + gcc_checking_assert (GET_CODE (loc_addr) == VALUE); > + if (canonical_cselib_val (CSELIB_VAL_PTR (loc_addr)) == addr) > + return true; > + } > + return false; > +} > + > /* Subroutine of cselib_lookup. Return a value for X, which is a MEM rtx. > If CREATE, make a new one if we haven't seen it before. */ > > @@ -1793,16 +1811,12 @@ cselib_lookup_mem (rtx x, int create) > addr = canonical_cselib_val (addr); > > /* Find a value that describes a value of our mode at that address. */ > - addr_space_t as = MEM_ADDR_SPACE (x); > for (elt_list *l = addr->addr_list; l; l = l->next) > - if (GET_MODE (l->elt->val_rtx) == mode) > + if (GET_MODE (l->elt->val_rtx) == mode > + && cselib_has_mem_loc_p (l->elt, addr, MEM_ADDR_SPACE (x))) > { > - for (elt_loc_list *l2 = l->elt->locs; l2; l2 = l2->next) > - if (MEM_P (l2->loc) && MEM_ADDR_SPACE (l2->loc) == as) > - { > - promote_debug_loc (l->elt->locs); > - return l->elt; > - } > + promote_debug_loc (l->elt->locs); > + return l->elt; > } > > if (! create) > diff --git a/gcc/cselib.h b/gcc/cselib.h > index 6493303f7..42908c81f 100644 > --- a/gcc/cselib.h > +++ b/gcc/cselib.h > @@ -41,7 +41,8 @@ struct cselib_val > struct elt_loc_list *locs; > > /* If this value is used as an address, points to a list of values that > - use it as an address in a MEM. */ > + use it as an address in a MEM. The MEMs can be in any address space. > + Each MEM, with its address space, is in the locs of its value. */ > struct elt_list *addr_list; > > struct cselib_val *next_containing_mem; > diff --git a/gcc/testsuite/gcc.target/i386/addr-space-9.c > b/gcc/testsuite/gcc.target/i386/addr-space-9.c > new file mode 100644 > index 000000000..93e93f0b2 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/addr-space-9.c > @@ -0,0 +1,19 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fno-dse" } */ > +/* { dg-additional-options "-fno-pic" { target ia32 } } */ > + > +/* *P and *Q are different memory locations with the same address. The > + stored value is also in TAB, a generic memory location. The load from > + Q must not reuse that value. */ > + > +static const long tab[4] = { 1, 2, 3, 4 }; > + > +long > +f (int i, long __seg_gs *p) > +{ > + long *q = (long *) (__UINTPTR_TYPE__) p; > + *p = tab[i]; > + return *q; > +} > + > +/* { dg-final { scan-assembler-times {\tmov[lq]\t\(%[a-z0-9]+\), %} 1 } } */ > -- > 2.43.0 >
