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
