On 10/21/2012 05:27 AM, Richard Sandiford wrote:
Hi Vlad,
Richard, sorry for long delay with the answer. I was really busy all
these days trying to fix a lot of GCC testsuite failures (for some ones
it was extremely difficult to find failure reasons) and push LRA into trunk.
Vladimir Makarov <vmaka...@redhat.com> writes:
@@ -543,23 +544,34 @@ extract_loc_address_regs (bool top_p, en
code1 = GET_CODE (arg1);
}
+ if (CONSTANT_P (arg0)
+ || code1 == PLUS || code1 == MULT || code1 == ASHIFT)
+ {
+ tloc = arg1_loc;
+ arg1_loc = arg0_loc;
+ arg0_loc = tloc;
+ arg0 = *arg0_loc;
+ code0 = GET_CODE (arg0);
+ arg1 = *arg1_loc;
+ code1 = GET_CODE (arg1);
+ }
When does this happen? Is it from the extract_address_regs calls
in equiv_address_substitution, or somewhere else?
I did not analyzed this. Sorry, I had no time for this. I just saw it
on a broken test.
I think the future work would be a support of canonical expressions
through all LRA and machine depended code and also address extract
rewriting when we figure out the exact syntax of the addresses.
/* If this machine only allows one register per address, it
must be in the first operand. */
if (MAX_REGS_PER_ADDRESS == 1 || code == LO_SUM)
{
- extract_loc_address_regs (false, mode, as, arg0_loc, false, code,
- code1, modify_p, ad);
lra_assert (ad->disp_loc == NULL);
ad->disp_loc = arg1_loc;
+ extract_loc_address_regs (false, mode, as, arg0_loc, false, code,
+ code1, modify_p, ad);
}
/* Base + disp addressing */
- else if (code != PLUS && code0 != MULT && code0 != ASHIFT
+ else if (code0 != PLUS && code0 != MULT && code0 != ASHIFT
&& CONSTANT_P (arg1))
{
- extract_loc_address_regs (false, mode, as, arg0_loc, false, PLUS,
- code1, modify_p, ad);
lra_assert (ad->disp_loc == NULL);
ad->disp_loc = arg1_loc;
+ extract_loc_address_regs (false, mode, as, arg0_loc, false, PLUS,
+ code1, modify_p, ad);
}
/* If index and base registers are the same on this machine,
just record registers in any non-constant operands. We
Wasn't sure: is the order of the disp_loc assignment and
extract_loc_address_regs call significant here, or did you just swap
them for cosmetic reasons? The original order seemed stronger on the
face of it, because we assert that the recursive call hasn't also
installed a displacement.
It was not a cosmetic change. It was a necessary fix for a broken test.
@@ -624,11 +635,18 @@ extract_loc_address_regs (bool top_p, en
case MULT:
case ASHIFT:
- extract_loc_address_regs (false, mode, as, &XEXP (*loc, 0), true,
- outer_code, code, modify_p, ad);
- lra_assert (ad->index_loc == NULL);
- ad->index_loc = loc;
- break;
+ {
+ rtx *arg0_loc = &XEXP (x, 0);
+ enum rtx_code code0 = GET_CODE (*arg0_loc);
+
+ if (code0 == CONST_INT)
+ arg0_loc = &XEXP (x, 1);
+ extract_loc_address_regs (false, mode, as, arg0_loc, true,
+ outer_code, code, modify_p, ad);
+ lra_assert (ad->index_loc == NULL);
+ ad->index_loc = loc;
+ break;
+ }
Is this just for the MULT case? Treating X in (ashift 1 X) as an
index register seems odd on the face of it.
Yes, right. It is really safe but it should be fixed for code clearness.
@@ -1757,10 +1786,10 @@ process_alt_operands (int only_alternati
clobber operand if the matching operand is
not dying in the insn. */
if (! curr_static_id->operand[m].early_clobber
- /*|| operand_reg[nop] == NULL_RTX
+ || operand_reg[nop] == NULL_RTX
|| (find_regno_note (curr_insn, REG_DEAD,
REGNO (operand_reg[nop]))
- != NULL_RTX)*/)
+ != NULL_RTX))
match_p = true;
}
if (match_p)
Ah, so did you find the reason why this was done? :-)
I already wrote you that I am going to restore the code, as I really
remember that I added this for purpose to fix some bug on some target.
It is better to have a safer code in the trunk. My intention is remove
this code again on the branch to figure out details why I did it. In
any case, even if this code is necessary, the code should be rewritten
to match when the operand is the same independently on presence of
REG_DEAD note.
Index: lra.c
===================================================================
--- lra.c (revision 192634)
+++ lra.c (working copy)
@@ -2035,7 +2035,8 @@ check_rtl (bool final_p)
as legitimate. Although they are legitimate if
they satisfies the constraints and will be checked
by insn constraints which we ignore here. */
- && GET_RTX_CLASS (GET_CODE (XEXP (op, 0))) == RTX_AUTOINC)
+ && GET_CODE (XEXP (op, 0)) != UNSPEC
+ && GET_RTX_CLASS (GET_CODE (XEXP (op, 0))) != RTX_AUTOINC)
fatal_insn_not_found (insn);
}
}
Which case was this needed for? RTX_AUTOINC codes really did seem like
a special case because they have their own constraints ("<" and ">").
But why do we have to assume that all unspec addresses are inherently valid?
That was a necessary fix for GCC testsuite regressions on x86/x86-64.
The check failed when final_p == false in order words before LRA
actually starts its job.
