This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Aliasing patch
- To: <gcc-patches at gcc dot gnu dot org>
- Subject: Aliasing patch
- From: Richard Sandiford <rsandifo at redhat dot com>
- Date: Fri, 16 Mar 2001 21:03:21 +0000 (GMT)
I was looking at the following alias bug in an old(ish) version of gcc. My
recent loop_regs_update patch is needed to expose it in the current sources
(a good reason not to apply it, I guess.) Without the loop_regs_update
patch, record_base_value is rarely called when the loop optimiser adds
new ivs:
http://gcc.gnu.org/ml/gcc-patches/2001-03/msg01166.html
This is the first time I've looked at the alias code, so apologies if the
description's a bit long.
The problem shows up on the mips, and I guess on others whose addressing
modes encourage the same sort of strength reduction. The main bit of
the test case is:
int a[] = { 1, 2, 3, 4, 5, 6 };
for (i = 0; i < limit1; i++)
{
for (j = 0; j < limit2; j++)
a[j+1] += a[j];
a[0] = 70;
}
The address of array "a" is (reg 1). During strength reduction, the
compiler creates a new iv that holds the address &a[j+1]. The iv (pseudo
119) is initialised to (reg 1) + 4 just before the inner loop. The alias
code is told about the initialisation through a call to record_base_value():
record_base_value (119, VAL)
where VAL = (plus (reg 1) (const_int 4)));
record_base_value does:
reg_base_value[119] = find_base_value (VAL);
If, instead of (reg 1), we had a pseudo base such as (reg 110), then
find_base_value() would return reg_base_value[110]. But find_base_value()
deliberately doesn't look in reg_base_value[] when given a hard register:
it returns the register rtx instead. More on that below. After the
call to record_base_value(), we have:
reg_base_value[1] == (address ...)
reg_base_value[119] == (reg 1)
This causes problems because:
base_alias_check ((reg 1), (reg 119))
will compare reg_base_value[1] with reg_base_value[119], and return false.
Here, (reg 1) is the address of a[0]. By returning false,
base_alias_check() convinces the optimiser that it's safe to move the
assignment to a[0] outside the loop.
A comment in find_base_value() says:
/* If a pseudo has a known base value, return it. Do not do this
for hard regs since it can result in a circular dependency
chain for registers which have values at function entry. */
But there seems no danger of that happening when the function is called
from record_base_value(), since (at least according to its comment) it is
only called when the loop optimiser creates psuedos. Also:
record_base_value (119, (reg 1))
*will* set reg_base_value[119] to reg_base_value[1] because record_base_reg()
seems to treat REG arguments as a special case:
if (GET_CODE (val) == REG)
{
if (REGNO (val) < reg_base_value_size)
reg_base_value[regno] = reg_base_value[REGNO (val)];
return;
}
reg_base_value[regno] = find_base_value (val);
Is this for optimisation purposes?
The patch below passes an extra argument to find_base_value to tell it
whether it is called from record_base_value() or from record_set(). That
might be too short-sighted a fix: I don't know if there are cases when
record_set() needs the behaviour selected for record_base_value().
Anyway, the patch shows no regressions on mips-elf and bootstraps on an i686
cygwin. The testcase will break after my loop_regs_update() patch, but is
refixed by this one. Please install if it's OK.
Richard
2001-03-16 Richard Sandiford <rsandifo@redhat.com>
* alias.c (find_base_value): Add an extra parameter to say where
the function is called from. Look up a hard reg's reg_base_value[]
if called from record_base_value(). Pass the extra parameter to
recursive calls.
(record_set): Pass 0 as the extra parameter.
(record_base_value): Pass 1 as the extra parameter.
* testsuite/gcc.c-torture/execute/20010316-1.c: New testcase
Index: ./alias.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/alias.c,v
retrieving revision 1.118
diff -c -p -d -r1.118 alias.c
*** ./alias.c 2001/03/14 03:21:43 1.118
--- ./alias.c 2001/03/16 17:45:42
*************** static int base_alias_check PARAMS ((rt
*** 97,103 ****
enum machine_mode));
static int handled_component_p PARAMS ((tree));
static int can_address_p PARAMS ((tree));
! static rtx find_base_value PARAMS ((rtx));
static int mems_in_disjoint_alias_sets_p PARAMS ((rtx, rtx));
static int insert_subset_children PARAMS ((splay_tree_node, void*));
static tree find_base_decl PARAMS ((tree));
--- 97,103 ----
enum machine_mode));
static int handled_component_p PARAMS ((tree));
static int can_address_p PARAMS ((tree));
! static rtx find_base_value PARAMS ((rtx, int));
static int mems_in_disjoint_alias_sets_p PARAMS ((rtx, rtx));
static int insert_subset_children PARAMS ((splay_tree_node, void*));
static tree find_base_decl PARAMS ((tree));
*************** get_frame_alias_set ()
*** 691,701 ****
return set;
}
! /* Inside SRC, the source of a SET, find a base address. */
static rtx
! find_base_value (src)
register rtx src;
{
unsigned int regno;
switch (GET_CODE (src))
--- 691,703 ----
return set;
}
! /* Inside SRC, the source of a SET, find a base address. RECORDING_BASE
! is true if we are called from record_base_value(). */
static rtx
! find_base_value (src, recording_base)
register rtx src;
+ int recording_base;
{
unsigned int regno;
switch (GET_CODE (src))
*************** find_base_value (src)
*** 713,725 ****
if (regno < FIRST_PSEUDO_REGISTER && copying_arguments)
return new_reg_base_value[regno];
! /* If a pseudo has a known base value, return it. Do not do this
! for hard regs since it can result in a circular dependency
! chain for registers which have values at function entry.
The test above is not sufficient because the scheduler may move
a copy out of an arg reg past the NOTE_INSN_FUNCTION_BEGIN. */
! if (regno >= FIRST_PSEUDO_REGISTER
&& regno < reg_base_value_size
&& reg_base_value[regno])
return reg_base_value[regno];
--- 715,729 ----
if (regno < FIRST_PSEUDO_REGISTER && copying_arguments)
return new_reg_base_value[regno];
! /* If a pseudo has a known base value, return it. Do this for hard
! regs too if we were called from record_base_value(). But we must
! not do this for hard regs if called from record_set() since that
! could result in a circular dependency chain for registers which
! have values at function entry.
The test above is not sufficient because the scheduler may move
a copy out of an arg reg past the NOTE_INSN_FUNCTION_BEGIN. */
! if ((recording_base || regno >= FIRST_PSEUDO_REGISTER)
&& regno < reg_base_value_size
&& reg_base_value[regno])
return reg_base_value[regno];
*************** find_base_value (src)
*** 753,766 ****
a known value for it. */
if (GET_CODE (src_0) == REG)
{
! temp = find_base_value (src_0);
if (temp != 0)
src_0 = temp;
}
if (GET_CODE (src_1) == REG)
{
! temp = find_base_value (src_1);
if (temp!= 0)
src_1 = temp;
}
--- 757,770 ----
a known value for it. */
if (GET_CODE (src_0) == REG)
{
! temp = find_base_value (src_0, recording_base);
if (temp != 0)
src_0 = temp;
}
if (GET_CODE (src_1) == REG)
{
! temp = find_base_value (src_1, recording_base);
if (temp!= 0)
src_1 = temp;
}
*************** find_base_value (src)
*** 769,785 ****
If either operand is a symbol, then it is the base. If
either operand is a CONST_INT, then the other is the base. */
if (GET_CODE (src_1) == CONST_INT || CONSTANT_P (src_0))
! return find_base_value (src_0);
else if (GET_CODE (src_0) == CONST_INT || CONSTANT_P (src_1))
! return find_base_value (src_1);
/* This might not be necessary anymore:
If either operand is a REG that is a known pointer, then it
is the base. */
else if (GET_CODE (src_0) == REG && REG_POINTER (src_0))
! return find_base_value (src_0);
else if (GET_CODE (src_1) == REG && REG_POINTER (src_1))
! return find_base_value (src_1);
return 0;
}
--- 773,789 ----
If either operand is a symbol, then it is the base. If
either operand is a CONST_INT, then the other is the base. */
if (GET_CODE (src_1) == CONST_INT || CONSTANT_P (src_0))
! return find_base_value (src_0, recording_base);
else if (GET_CODE (src_0) == CONST_INT || CONSTANT_P (src_1))
! return find_base_value (src_1, recording_base);
/* This might not be necessary anymore:
If either operand is a REG that is a known pointer, then it
is the base. */
else if (GET_CODE (src_0) == REG && REG_POINTER (src_0))
! return find_base_value (src_0, recording_base);
else if (GET_CODE (src_1) == REG && REG_POINTER (src_1))
! return find_base_value (src_1, recording_base);
return 0;
}
*************** find_base_value (src)
*** 787,799 ****
case LO_SUM:
/* The standard form is (lo_sum reg sym) so look only at the
second operand. */
! return find_base_value (XEXP (src, 1));
case AND:
/* If the second operand is constant set the base
address to the first operand. */
if (GET_CODE (XEXP (src, 1)) == CONST_INT && INTVAL (XEXP (src, 1)) != 0)
! return find_base_value (XEXP (src, 0));
return 0;
case TRUNCATE:
--- 791,803 ----
case LO_SUM:
/* The standard form is (lo_sum reg sym) so look only at the
second operand. */
! return find_base_value (XEXP (src, 1), recording_base);
case AND:
/* If the second operand is constant set the base
address to the first operand. */
if (GET_CODE (XEXP (src, 1)) == CONST_INT && INTVAL (XEXP (src, 1)) != 0)
! return find_base_value (XEXP (src, 0), recording_base);
return 0;
case TRUNCATE:
*************** find_base_value (src)
*** 803,809 ****
case ZERO_EXTEND:
case SIGN_EXTEND: /* used for NT/Alpha pointers */
case HIGH:
! return find_base_value (XEXP (src, 0));
default:
break;
--- 807,813 ----
case ZERO_EXTEND:
case SIGN_EXTEND: /* used for NT/Alpha pointers */
case HIGH:
! return find_base_value (XEXP (src, 0), recording_base);
default:
break;
*************** record_set (dest, set, data)
*** 889,895 ****
else if (XEXP (src, 1) == dest)
other = XEXP (src, 0);
! if (! other || find_base_value (other))
new_reg_base_value[regno] = 0;
break;
}
--- 893,899 ----
else if (XEXP (src, 1) == dest)
other = XEXP (src, 0);
! if (! other || find_base_value (other, 0))
new_reg_base_value[regno] = 0;
break;
}
*************** record_set (dest, set, data)
*** 904,910 ****
/* If this is the first set of a register, record the value. */
else if ((regno >= FIRST_PSEUDO_REGISTER || ! fixed_regs[regno])
&& ! reg_seen[regno] && new_reg_base_value[regno] == 0)
! new_reg_base_value[regno] = find_base_value (src);
reg_seen[regno] = 1;
}
--- 908,914 ----
/* If this is the first set of a register, record the value. */
else if ((regno >= FIRST_PSEUDO_REGISTER || ! fixed_regs[regno])
&& ! reg_seen[regno] && new_reg_base_value[regno] == 0)
! new_reg_base_value[regno] = find_base_value (src, 0);
reg_seen[regno] = 1;
}
*************** record_base_value (regno, val, invariant
*** 935,941 ****
return;
}
! reg_base_value[regno] = find_base_value (val);
}
/* Returns a canonical version of X, from the point of view alias
--- 939,945 ----
return;
}
! reg_base_value[regno] = find_base_value (val, 1);
}
/* Returns a canonical version of X, from the point of view alias
*** ./testsuite/gcc.c-torture/execute/20010316-1.c.new Fri Mar 16 20:42:47 2001
--- ./testsuite/gcc.c-torture/execute/20010316-1.c Fri Mar 16 17:56:08 2001
***************
*** 0 ****
--- 1,27 ----
+
+ int
+ test (limit1, limit2)
+ int limit1, limit2;
+ {
+ int i, j;
+ int a[] = { 1, 2, 3, 4, 5, 6 };
+
+ for (i = 0; i < limit1; i++)
+ {
+ for (j = 0; j < limit2; j++)
+ a[j+1] += a[j];
+ a[0] = 70;
+ }
+ return a[1] == 73;
+ }
+
+ int
+ main ()
+ {
+ if (! test (2, 4))
+ abort ();
+ else
+ exit (0);
+ }
+
+