Hello!
Attached patch fixes PR63475, where postreload CSE propagates aliased
memory operand.
The core of the problem was with the call to base_alias_check when
VALUE RTXes are involved. Before the call, find_base_term is used to
extract the base of x_addr and mem_addr. Please note that
find_base_term is able to extract the bases from VALUE RTXes. These
extracted bases were passed to base_alias_check, together with
original VALUE RTXes x_addr and mem_addr.
The problem begins here. base_alias_check doesn't handle VALUE RTXes,
and uses e.g. canon_rtx on VALUEs and various GET_CODE accessors to
determine various properties of passed x_addr and mem_addr. One of
these check checks for the AND alignment addresses to prevent:
/* Differing symbols not accessed via AND never alias. */
if (GET_CODE (x_base) != ADDRESS && GET_CODE (y_base) != ADDRESS)
return 0;
early exit. However, when x and y are passed as VALUE RTXes (that
corresponds and hides the address with AND), and preceding calls to
find_base_term are nevertheless able to extract the bases of x and y,
this condition fires erroneously and invalid return value is returned
(with 0 meaning that the addresses X and Y are known to point to
different objects).
The solution is to always extract values for x_addr and mem_addr and
use them in the calls to find_base_term and base_alias_check.
[It can happen that get_addr is not able to match VALUE RTX with some
address, so it is not possible to simply add a bunch of "GET_CODE (x)
!= VALUE" asserts in base_alias_check. But in this case find_base_term
returns ADDRESS RTX, so we stay in sync as far as base_alias_check is
concerned (see the quoted code above).]
Added benefit of the patch is, that canon_rtx now works as expected.
canon_rtx does NOT handle VALUE RTXes.
A small optimization is also present. If the address is already
canonicalized, we pass original address to memrefs_conflict_p, but we
have to extract original address for preceding functions nevertheless.
Also, we use extracted original address in recently added check for
AND aligned addresses when checking for MEM_READONLY_P.
The patch also removes a couple of unneeded and unused calls to
canon_rtx, also to show the level of bitrot in this area ...
2014-10-14 Uros Bizjak <ubiz...@gmail.com>
PR rtl-optimization/63475
* alias.c (true_dependence_1): Always use get_addr to extract
true address operands from x_addr and mem_addr. Use extracted
address operands to check for references with alignment ANDs.
Use extracted address operands with find_base_term and
base_alis_check. For noncanonicalized operands call canon_rtx with
extracted address operand.
(write_dependence_1): Ditto.
(may_alias_p): Ditto. Remove unused calls to canon_rtx.
Patch was thoroughly tested on x86_64-linux-gnu {,-m32} and
alpha-linux-gnu for all default languages plus obj-c++ and go. While
there was no differences on x86_64-linux-gnu (as expected),
alpha-linux-gnu improved the result [1] for some hundred of PASSes in
gfortran testsuite [2].
OK for mainline?
[1] https://gcc.gnu.org/ml/gcc-testresults/2014-10/msg01151.html
[2] https://gcc.gnu.org/ml/gcc-testresults/2014-10/msg01478.html
Uros.
Index: alias.c
===================================================================
--- alias.c (revision 216149)
+++ alias.c (working copy)
@@ -2439,6 +2439,7 @@ static int
true_dependence_1 (const_rtx mem, enum machine_mode mem_mode, rtx mem_addr,
const_rtx x, rtx x_addr, bool mem_canonicalized)
{
+ rtx true_mem_addr;
rtx base;
int ret;
@@ -2458,6 +2459,10 @@ true_dependence_1 (const_rtx mem, enum machine_mod
|| MEM_ALIAS_SET (mem) == ALIAS_SET_MEMORY_BARRIER)
return 1;
+ if (! x_addr)
+ x_addr = XEXP (x, 0);
+ x_addr = get_addr (x_addr);
+
if (! mem_addr)
{
mem_addr = XEXP (mem, 0);
@@ -2464,23 +2469,8 @@ true_dependence_1 (const_rtx mem, enum machine_mod
if (mem_mode == VOIDmode)
mem_mode = GET_MODE (mem);
}
+ true_mem_addr = get_addr (mem_addr);
- if (! x_addr)
- {
- x_addr = XEXP (x, 0);
- if (!((GET_CODE (x_addr) == VALUE
- && GET_CODE (mem_addr) != VALUE
- && reg_mentioned_p (x_addr, mem_addr))
- || (GET_CODE (x_addr) != VALUE
- && GET_CODE (mem_addr) == VALUE
- && reg_mentioned_p (mem_addr, x_addr))))
- {
- x_addr = get_addr (x_addr);
- if (! mem_canonicalized)
- mem_addr = get_addr (mem_addr);
- }
- }
-
/* Read-only memory is by definition never modified, and therefore can't
conflict with anything. However, don't assume anything when AND
addresses are involved and leave to the code below to determine
@@ -2488,7 +2478,7 @@ true_dependence_1 (const_rtx mem, enum machine_mod
stupid user tricks can produce them, so don't die. */
if (MEM_READONLY_P (x)
&& GET_CODE (x_addr) != AND
- && GET_CODE (mem_addr) != AND)
+ && GET_CODE (true_mem_addr) != AND)
return 0;
/* If we have MEMs referring to different address spaces (which can
@@ -2503,14 +2493,14 @@ true_dependence_1 (const_rtx mem, enum machine_mod
&& CONSTANT_POOL_ADDRESS_P (base))))
return 0;
- rtx mem_base = find_base_term (mem_addr);
- if (! base_alias_check (x_addr, base, mem_addr, mem_base,
+ rtx mem_base = find_base_term (true_mem_addr);
+ if (! base_alias_check (x_addr, base, true_mem_addr, mem_base,
GET_MODE (x), mem_mode))
return 0;
x_addr = canon_rtx (x_addr);
if (!mem_canonicalized)
- mem_addr = canon_rtx (mem_addr);
+ mem_addr = canon_rtx (true_mem_addr);
if ((ret = memrefs_conflict_p (GET_MODE_SIZE (mem_mode), mem_addr,
SIZE_FOR_MODE (x), x_addr, 0)) != -1)
@@ -2560,6 +2550,7 @@ write_dependence_p (const_rtx mem,
bool mem_canonicalized, bool x_canonicalized, bool writep)
{
rtx mem_addr;
+ rtx true_mem_addr, true_x_addr;
rtx base;
int ret;
@@ -2580,30 +2571,20 @@ write_dependence_p (const_rtx mem,
|| MEM_ALIAS_SET (mem) == ALIAS_SET_MEMORY_BARRIER)
return 1;
- mem_addr = XEXP (mem, 0);
if (!x_addr)
- {
- x_addr = XEXP (x, 0);
- if (!((GET_CODE (x_addr) == VALUE
- && GET_CODE (mem_addr) != VALUE
- && reg_mentioned_p (x_addr, mem_addr))
- || (GET_CODE (x_addr) != VALUE
- && GET_CODE (mem_addr) == VALUE
- && reg_mentioned_p (mem_addr, x_addr))))
- {
- x_addr = get_addr (x_addr);
- if (!mem_canonicalized)
- mem_addr = get_addr (mem_addr);
- }
- }
+ x_addr = XEXP (x, 0);
+ true_x_addr = get_addr (x_addr);
+ mem_addr = XEXP (mem, 0);
+ true_mem_addr = get_addr (mem_addr);
+
/* A read from read-only memory can't conflict with read-write memory.
Don't assume anything when AND addresses are involved and leave to
the code below to determine dependence. */
if (!writep
&& MEM_READONLY_P (mem)
- && GET_CODE (x_addr) != AND
- && GET_CODE (mem_addr) != AND)
+ && GET_CODE (true_x_addr) != AND
+ && GET_CODE (true_mem_addr) != AND)
return 0;
/* If we have MEMs referring to different address spaces (which can
@@ -2612,7 +2593,7 @@ write_dependence_p (const_rtx mem,
if (MEM_ADDR_SPACE (mem) != MEM_ADDR_SPACE (x))
return 1;
- base = find_base_term (mem_addr);
+ base = find_base_term (true_mem_addr);
if (! writep
&& base
&& (GET_CODE (base) == LABEL_REF
@@ -2620,18 +2601,18 @@ write_dependence_p (const_rtx mem,
&& CONSTANT_POOL_ADDRESS_P (base))))
return 0;
- rtx x_base = find_base_term (x_addr);
- if (! base_alias_check (x_addr, x_base, mem_addr, base, GET_MODE (x),
- GET_MODE (mem)))
+ rtx x_base = find_base_term (true_x_addr);
+ if (! base_alias_check (true_x_addr, x_base, true_mem_addr, base,
+ GET_MODE (x), GET_MODE (mem)))
return 0;
if (!x_canonicalized)
{
- x_addr = canon_rtx (x_addr);
+ x_addr = canon_rtx (true_x_addr);
x_mode = GET_MODE (x);
}
if (!mem_canonicalized)
- mem_addr = canon_rtx (mem_addr);
+ mem_addr = canon_rtx (true_mem_addr);
if ((ret = memrefs_conflict_p (SIZE_FOR_MODE (mem), mem_addr,
GET_MODE_SIZE (x_mode), x_addr, 0)) != -1)
@@ -2700,17 +2681,10 @@ may_alias_p (const_rtx mem, const_rtx x)
return 1;
x_addr = XEXP (x, 0);
+ x_addr = get_addr (x_addr);
+
mem_addr = XEXP (mem, 0);
- if (!((GET_CODE (x_addr) == VALUE
- && GET_CODE (mem_addr) != VALUE
- && reg_mentioned_p (x_addr, mem_addr))
- || (GET_CODE (x_addr) != VALUE
- && GET_CODE (mem_addr) == VALUE
- && reg_mentioned_p (mem_addr, x_addr))))
- {
- x_addr = get_addr (x_addr);
- mem_addr = get_addr (mem_addr);
- }
+ mem_addr = get_addr (mem_addr);
/* Read-only memory is by definition never modified, and therefore can't
conflict with anything. However, don't assume anything when AND
@@ -2734,9 +2708,6 @@ may_alias_p (const_rtx mem, const_rtx x)
GET_MODE (x), GET_MODE (mem_addr)))
return 0;
- x_addr = canon_rtx (x_addr);
- mem_addr = canon_rtx (mem_addr);
-
if (nonoverlapping_memrefs_p (mem, x, true))
return 0;
