From: Kyrylo Tkachov <[email protected]>
cselib_lookup_mem looks for a value of the mode of X on the addr_list of
the value of the address of X. It accepts a value if one of its
locations is a MEM in the address space of X, but it does not check that
this MEM is at the address of X. A store in another address space can
put a value on the addr_list, and the value can also have a location at
another address in the address space of X. 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 %gs store puts the value of TAB[I] on the addr_list of P. The
generic load from Q then finds it through its location in TAB, and
postreload 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
Require the MEM location to be at the address of X too.
Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-linux.
Ok for trunk?
Thanks,
Kyrill
Cc: Jeff Law <[email protected]>
gcc/ChangeLog:
* cselib.cc (cselib_has_mem_loc_p): New function.
(cselib_lookup_mem): Use it to also check the address of the
memory location.
gcc/testsuite/ChangeLog:
* gcc.target/i386/addr-space-9.c: New test.
Signed-off-by: Kyrylo Tkachov <[email protected]>
---
New in v2. Patch 2/4 uses the same check for its new query. v1 of
that patch had the same problem.
gcc/cselib.cc | 30 ++++++++++++++------
gcc/testsuite/gcc.target/i386/addr-space-9.c | 19 +++++++++++++
2 files changed, 41 insertions(+), 8 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/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
