This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Reload problem & fix
- To: gcc-patches at gcc dot gnu dot org
- Subject: Reload problem & fix
- From: Bernd Schmidt <bernds at redhat dot co dot uk>
- Date: Mon, 23 Oct 2000 18:12:48 +0100 (BST)
- cc: jakub at redhat dot com
The testcase below fails to compile properly with current CVS on i586-linux
with -O2 -fomit-frame-pointer. An output reload is incorrectly deleted.
In the .lreg dump, we have
(insn 118 116 120 (set (reg/v:QI 44)
(mem/s:QI (plus:SI (reg:SI 71)
(reg/v:SI 42)) 0)) 44 {*movqi_1} (insn_list 114 (nil))
(insn 120 118 124 (parallel[
(set (reg/v:QI 44)
(xor:QI (mem/s:QI (plus:SI (reg:SI 70)
(reg/v:SI 42)) 0)
(reg/v:QI 44)))
(clobber (reg:CC 17 flags))
This gets turned into
(insn 560 116 118 (set (reg:QI 0 al)
(mem/s:QI (plus:SI (reg:SI 3 ebx)
(reg/v:SI 5 edi)) 0)) 44 {*movqi_1} (nil)
(note 118 560 563 NOTE_INSN_DELETED 1075285216)
(insn 563 118 120 (set (reg:QI 0 al)
(mem/s:QI (plus:SI (reg:SI 1 edx)
(reg/v:SI 5 edi)) 0)) 44 {*movqi_1} (nil)
(insn 120 563 124 (parallel[
(set (mem:QI (plus:SI (reg:SI 7 esp)
(const_int 31 [0x1f])) 10)
(xor:QI (mem:QI (plus:SI (reg:SI 7 esp)
(const_int 31 [0x1f])) 10)
(reg:QI 0 al)))
(clobber (reg:CC 17 flags))
after reload. Note that insn 118 has been deleted by delete_output_reload.
This is done because we have a bogus optional output reload for insn 120:
Reloads for insn # 120
Reload 0: reload_out (QI) = (mem:QI (plus:SI (reg:SI 7 esp)
(const_int 31 [0x1f])) 10)
NO_REGS, RELOAD_FOR_OUTPUT (opnum = 0), optional
reload_out_reg: (reg/v:QI 44)
which is incorrect, since it ought to have been an in-out reload - operands 0
and 1 must match in the insn. Thus, in do_output_reload, we walk into the part
that calls delete_output_reload.
The incorrect output reload gets emitted because during find_reloads,
goal_alternative_matched[0] has the value -1. This variable should, for any
operand X, normally contain the operand number of another operand that must
match X. In this case, it did not get set up properly.
The code to set up goal_alternative_matched is this:
for (i = 0; i < noperands; i++)
goal_alternative_matched[i] = -1;
for (i = 0; i < noperands; i++)
if (! goal_alternative_win[i]
&& goal_alternative_matches[i] >= 0)
goal_alternative_matched[goal_alternative_matches[i]] = i;
I originally suspected that there was something fishy going on with
commutative operand switching (note how in this example, operands 1 & 2 of
insn 120 get swapped, but I couldn't find anything wrong with it after
staring at it for a while. I believe the problem is rather the use of
goal_alternative_win here.
I *assume* that we're testing goal_alternative_win to weed out cases where
the matching constraint is just one possibility in a given alternative, e.g.
(set (match_operand 0 ... "m") (foo (match_operand 1 ... "0r"))
In that case, if operand 0 matches the 'm' constraint and operand 1 matches
the 'r' constraint, then both operands win, but they don't need to match.
However, this ignores the fact that if operand 1 does _not_ match the 'r'
constraint, but _does_ match operand 0, then goal_alternative_win is also
set to 1.
It seems that this means we have to distinguish between the case where an
operand wins "normally", and the case where it wins because it can match
another operand. That's what the patch below does. It survived a bootstrap
on i686-linux.
Bernd
unsigned char a[256], b[256], c[256], d[256];
void foo(unsigned char *x, int y, unsigned char *z)
{
}
void bar(int x, ...)
{
}
void baz(int y)
{
if (y != 0x10)
abort();
}
void test(int x, unsigned char *y)
{
unsigned char g,h,j, k[5],l[5], m[30];
int i;
bar(x, y[0], y[1], y[2], y[3], y[4], y[5], y[6], y[7], y[8], y[9]);
for (i = 5; --i >= 0; )
k[i] = y[5 + i] ^ a[i] ^ c[i];
foo(&m[29], sizeof m, k);
g = d[x] ^ c[x];
bar(x, d[x], x, c[x]);
baz(g);
for (i = 5, h = 0; --i >= 0; h = y[i])
{
j = m[25 + i] ^ y[i];
j = b[j] ^ g;
k[i] = c[j] ^ h;
}
for (i = 5, h = 0; --i >= 0; h = k[i])
{
j = m[20 + i] ^ k[i];
j = b[j] ^ g;
l[i] = c[j] ^ h;
}
for (i = 5, h = 0; --i >= 0; h = l[i]) {
j = m[15 + i] ^ l[i];
j = b[j] ^ g;
j = c[j] ^ h;
k[i] = a[j] ^ c[j];
}
}
int main()
{
c[4] = 0xdc;
d[4] = 0xcc;
test(4, a);
exit(0);
}
* reload.c (find_reloads): Distinguish "wins" so that we know whether
a given operand won because of a matching constraint or not; then use
that information to compute goal_alternative_matched properly.
Index: reload.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload.c,v
retrieving revision 1.130
diff -u -p -r1.130 reload.c
--- reload.c 2000/09/14 17:42:48 1.130
+++ reload.c 2000/10/23 16:05:19
@@ -2399,6 +2399,7 @@ find_reloads (insn, replace, ind_levels,
int no_input_reloads = 0, no_output_reloads = 0;
int n_alternatives;
int this_alternative[MAX_RECOG_OPERANDS];
+ char this_alternative_match_win[MAX_RECOG_OPERANDS];
char this_alternative_win[MAX_RECOG_OPERANDS];
char this_alternative_offmemok[MAX_RECOG_OPERANDS];
char this_alternative_earlyclobber[MAX_RECOG_OPERANDS];
@@ -2410,6 +2411,7 @@ find_reloads (insn, replace, ind_levels,
int operand_reloadnum[MAX_RECOG_OPERANDS];
int goal_alternative_matches[MAX_RECOG_OPERANDS];
int goal_alternative_matched[MAX_RECOG_OPERANDS];
+ char goal_alternative_match_win[MAX_RECOG_OPERANDS];
char goal_alternative_win[MAX_RECOG_OPERANDS];
char goal_alternative_offmemok[MAX_RECOG_OPERANDS];
char goal_alternative_earlyclobber[MAX_RECOG_OPERANDS];
@@ -2741,6 +2743,7 @@ find_reloads (insn, replace, ind_levels,
{
register char *p = constraints[i];
register int win = 0;
+ int did_match = 0;
/* 0 => this operand can be reloaded somehow for this alternative */
int badop = 1;
/* 0 => this operand can be reloaded if the alternative allows regs. */
@@ -2839,6 +2842,7 @@ find_reloads (insn, replace, ind_levels,
this_alternative[i] = (int) NO_REGS;
this_alternative_win[i] = 0;
+ this_alternative_match_win[i] = 0;
this_alternative_offmemok[i] = 0;
this_alternative_earlyclobber[i] = 0;
this_alternative_matches[i] = -1;
@@ -2917,7 +2921,7 @@ find_reloads (insn, replace, ind_levels,
&& ! this_alternative_win[c])
bad = 1;
- win = this_alternative_win[c];
+ did_match = this_alternative_win[c];
}
else
{
@@ -2953,12 +2957,11 @@ find_reloads (insn, replace, ind_levels,
operand also had to match the same thing as this
operand, we don't know how to do that. So reject this
alternative. */
- if (! win || force_reload)
+ if (! did_match || force_reload)
for (j = 0; j < i; j++)
if (this_alternative_matches[j]
== this_alternative_matches[i])
badop = 1;
-
break;
case 'p':
@@ -3175,6 +3178,8 @@ find_reloads (insn, replace, ind_levels,
this_alternative_earlyclobber[i] = earlyclobber;
if (win && ! force_reload)
this_alternative_win[i] = 1;
+ else if (did_match && ! force_reload)
+ this_alternative_match_win[i] = 1;
else
{
int const_to_mem = 0;
@@ -3276,7 +3281,8 @@ find_reloads (insn, replace, ind_levels,
Don't do this if the preferred class has only one register
because we might otherwise exhaust the class. */
- if (! win && this_alternative[i] != (int) NO_REGS
+ if (! win && ! did_match
+ && this_alternative[i] != (int) NO_REGS
&& GET_MODE_SIZE (operand_mode[i]) <= UNITS_PER_WORD
&& reg_class_size[(int) preferred_class[i]] > 1)
{
@@ -3302,7 +3308,7 @@ find_reloads (insn, replace, ind_levels,
for (i = 0; i < noperands; i++)
if (this_alternative_earlyclobber[i]
- && this_alternative_win[i])
+ && (this_alternative_win[i] || this_alternative_match_win[i]))
{
struct decomposition early_data;
@@ -3345,6 +3351,7 @@ find_reloads (insn, replace, ind_levels,
{
losers++;
this_alternative_win[j] = 0;
+ this_alternative_match_win[j] = 0;
}
else
break;
@@ -3355,11 +3362,13 @@ find_reloads (insn, replace, ind_levels,
{
losers++;
this_alternative_win[i] = 0;
+ this_alternative_match_win[j] = 0;
for (j = 0; j < noperands; j++)
if (this_alternative_matches[j] == i
- && this_alternative_win[j])
+ && this_alternative_match_win[j])
{
this_alternative_win[j] = 0;
+ this_alternative_match_win[j] = 0;
losers++;
}
}
@@ -3378,7 +3387,8 @@ find_reloads (insn, replace, ind_levels,
}
for (i = 0; i < noperands; i++)
{
- goal_alternative_win[i] = 1;
+ goal_alternative_win[i] = this_alternative_win[i];
+ goal_alternative_match_win[i] = this_alternative_match_win[i];
goal_alternative[i] = this_alternative[i];
goal_alternative_offmemok[i] = this_alternative_offmemok[i];
goal_alternative_matches[i] = this_alternative_matches[i];
@@ -3406,6 +3416,7 @@ find_reloads (insn, replace, ind_levels,
{
goal_alternative[i] = this_alternative[i];
goal_alternative_win[i] = this_alternative_win[i];
+ goal_alternative_match_win[i] = this_alternative_match_win[i];
goal_alternative_offmemok[i] = this_alternative_offmemok[i];
goal_alternative_matches[i] = this_alternative_matches[i];
goal_alternative_earlyclobber[i]
@@ -3487,11 +3498,14 @@ find_reloads (insn, replace, ind_levels,
for (i = 0; i < noperands; i++)
goal_alternative_matched[i] = -1;
-
+
for (i = 0; i < noperands; i++)
if (! goal_alternative_win[i]
&& goal_alternative_matches[i] >= 0)
goal_alternative_matched[goal_alternative_matches[i]] = i;
+
+ for (i = 0; i < noperands; i++)
+ goal_alternative_win[i] |= goal_alternative_match_win[i];
/* If the best alternative is with operands 1 and 2 swapped,
consider them swapped before reporting the reloads. Update the