Robert Suchanek <[email protected]> writes:
>> >> Did you see the failures even after your mips_regno_mode_ok_for_base_p
>> >> change? LRA should know how to reload a "W" address.
>> >
>> > Yes but I realize there is more. It fails because $sp is now included
>> > in BASE_REG_CLASS and "W" is based on it. However, I suppose that
>> > it would be too eager to say it is wrong and likely there is
>> > something missing
>> > in LRA if we want to keep all alternatives. Currently there is no check
>> > if a reloaded operand has a valid address, use of $sp in lbu/lhu cases.
>> > Even if we added extra checks we are less likely to benefit as we need
>> > to reload the base into register.
>>
>> Not sure what you mean, sorry. "W" exists specifically to exclude
>> $sp-based and $pc-based addresses. LRA AFAIK should already be able
>> to reload addresses that are valid in the TARGET_LEGITIMATE_ADDRESS_P
>> sense but which do not match the constraints for a particular insn.
>>
>> Can you remember one of the tests that fails?
>
> I couldn't trigger the problem with the original testcase but found
> another one that reveals it. The following needs to compiled with
> -mips32r2 -mips16 -Os:
>
> struct { int addr; } c;
> struct command { int args[1]; };
> unsigned short a;
>
> fn1 (struct command *p1)
> {
> unsigned short d;
> d = fn2 ();
> a = p1->args[0];
> fn3 (a);
> if (c.addr)
> {
> fn4 (p1->args[0]);
> return;
> }
> (&c)->addr = fn5 ();
> fn6 (d);
> }
Thanks.
> Not sure how the constraint would/should exclude $sp-based address in
> LRA. In this particular case, a spilled pseudo is changed to memory
> giving the following RTL form:
>
> (insn 30 29 31 4 (set (reg:SI 4 $4)
> (and:SI (mem/c:SI (plus:SI (reg/f:SI 78 $frame)
> (const_int 16 [0x10])) [7 %sfp+16 S4 A32])
> (const_int 65535 [0xffff]))) shell.i:17 161 {*andsi3_mips16}
> (expr_list:REG_DEAD (reg:SI 194 [ D.1469 ])
> (nil)))
>
> The operand 1 during alternative selection is not marked as a bad
> operand as it is a memory operand. $frame appears to be fine as it
> could be eliminated later to hard register. No reloads are inserted
> for the instructions concerned. Unless, $frame should be temporarily
> eliminated and then a reload would be inserted?
Yeah, I think the lack of elimination is the problem. process_address
eliminates $frame temporarily before checking whether the address
is valid, but the places that check EXTRA_CONSTRAINT_STR pass the
original uneliminated address. So the legitimate_address_p hook sees
the $sp-based address but the "W" constraint only sees the $frame-based
address (which might or might not be valid, depending on whether $frame
is eliminated to the stack or hard frame pointer). I think the constraints
should see the eliminated address too.
This patch seems to fix it for me. Tested on x86_64-linux-gnu.
Vlad, is this OK for trunk?
BTW, we might want to define something like:
#define MODE_BASE_REG_CLASS(MODE) \
(TARGET_MIPS16 \
? ((MODE) == SImode || (MODE) == DImode ? M16_SP_REGS : M16_REGS) \
: GR_REGS)
instead of BASE_REG_CLASS. It might lead to slightly better code
(or not -- if it doesn't then don't bother :-)).
If this patch is OK then I think the only thing blocking the switch
to LRA is the asm-subreg-1.c failure. I think it'd be fine to XFAIL
that test on MIPS for now, until there's a consensus about what "X" means
for asms.
Thanks,
Richard
gcc/
* lra-constraints.c (valid_address_p): Move earlier in file.
Add a constraint argument to the address_info version.
(satisfies_memory_constraint_p): New function.
(satisfies_address_constraint_p): Likewise.
(process_alt_operands, curr_insn_transform): Use them.
(process_address): Pass the constraint to valid_address_p when
checking address operands.
Index: gcc/lra-constraints.c
===================================================================
--- gcc/lra-constraints.c 2014-04-21 10:36:06.715026374 +0100
+++ gcc/lra-constraints.c 2014-04-21 13:18:46.228298176 +0100
@@ -317,6 +317,94 @@ in_mem_p (int regno)
return get_reg_class (regno) == NO_REGS;
}
+/* Return 1 if ADDR is a valid memory address for mode MODE in address
+ space AS, and check that each pseudo has the proper kind of hard
+ reg. */
+static int
+valid_address_p (enum machine_mode mode ATTRIBUTE_UNUSED,
+ rtx addr, addr_space_t as)
+{
+#ifdef GO_IF_LEGITIMATE_ADDRESS
+ lra_assert (ADDR_SPACE_GENERIC_P (as));
+ GO_IF_LEGITIMATE_ADDRESS (mode, addr, win);
+ return 0;
+
+ win:
+ return 1;
+#else
+ return targetm.addr_space.legitimate_address_p (mode, addr, 0, as);
+#endif
+}
+
+/* Return whether address AD is valid. If CONSTRAINT is null,
+ check for general addresses, otherwise check the extra constraint
+ CONSTRAINT. */
+static bool
+valid_address_p (struct address_info *ad, const char *constraint = 0)
+{
+ /* Some ports do not check displacements for eliminable registers,
+ so we replace them temporarily with the elimination target. */
+ rtx saved_base_reg = NULL_RTX;
+ rtx saved_index_reg = NULL_RTX;
+ rtx *base_term = strip_subreg (ad->base_term);
+ rtx *index_term = strip_subreg (ad->index_term);
+ if (base_term != NULL)
+ {
+ saved_base_reg = *base_term;
+ lra_eliminate_reg_if_possible (base_term);
+ if (ad->base_term2 != NULL)
+ *ad->base_term2 = *ad->base_term;
+ }
+ if (index_term != NULL)
+ {
+ saved_index_reg = *index_term;
+ lra_eliminate_reg_if_possible (index_term);
+ }
+ bool ok_p = (constraint
+#ifdef EXTRA_CONSTRAINT_STR
+ ? EXTRA_CONSTRAINT_STR (*ad->outer, constraint[0], constraint)
+#else
+ ? false
+#endif
+ : valid_address_p (ad->mode, *ad->outer, ad->as));
+ if (saved_base_reg != NULL_RTX)
+ {
+ *base_term = saved_base_reg;
+ if (ad->base_term2 != NULL)
+ *ad->base_term2 = *ad->base_term;
+ }
+ if (saved_index_reg != NULL_RTX)
+ *index_term = saved_index_reg;
+ return ok_p;
+}
+
+#ifdef EXTRA_CONSTRAINT_STR
+/* Return true if, after elimination, OP satisfies extra memory constraint
+ CONSTRAINT. */
+static bool
+satisfies_memory_constraint_p (rtx op, const char *constraint)
+{
+ struct address_info ad;
+
+ if (!MEM_P (op))
+ return false;
+
+ decompose_mem_address (&ad, op);
+ return valid_address_p (&ad, constraint);
+}
+
+/* Return true if, after elimination, OP satisfies extra address constraint
+ CONSTRAINT. */
+static bool
+satisfies_address_constraint_p (rtx op, const char *constraint)
+{
+ struct address_info ad;
+
+ decompose_lea_address (&ad, &op);
+ return valid_address_p (&ad, constraint);
+}
+#endif
+
/* Initiate equivalences for LRA. As we keep original equivalences
before any elimination, we need to make copies otherwise any change
in insns might change the equivalences. */
@@ -1941,7 +2029,7 @@ process_alt_operands (int only_alternati
#ifdef EXTRA_CONSTRAINT_STR
if (EXTRA_MEMORY_CONSTRAINT (c, p))
{
- if (EXTRA_CONSTRAINT_STR (op, c, p))
+ if (satisfies_memory_constraint_p (op, p))
win = true;
else if (spilled_pseudo_p (op))
win = true;
@@ -1960,7 +2048,7 @@ process_alt_operands (int only_alternati
}
if (EXTRA_ADDRESS_CONSTRAINT (c, p))
{
- if (EXTRA_CONSTRAINT_STR (op, c, p))
+ if (satisfies_address_constraint_p (op, p))
win = true;
/* If we didn't already win, we can reload
@@ -2576,60 +2664,6 @@ process_alt_operands (int only_alternati
return ok_p;
}
-/* Return 1 if ADDR is a valid memory address for mode MODE in address
- space AS, and check that each pseudo has the proper kind of hard
- reg. */
-static int
-valid_address_p (enum machine_mode mode ATTRIBUTE_UNUSED,
- rtx addr, addr_space_t as)
-{
-#ifdef GO_IF_LEGITIMATE_ADDRESS
- lra_assert (ADDR_SPACE_GENERIC_P (as));
- GO_IF_LEGITIMATE_ADDRESS (mode, addr, win);
- return 0;
-
- win:
- return 1;
-#else
- return targetm.addr_space.legitimate_address_p (mode, addr, 0, as);
-#endif
-}
-
-/* Return whether address AD is valid. */
-
-static bool
-valid_address_p (struct address_info *ad)
-{
- /* Some ports do not check displacements for eliminable registers,
- so we replace them temporarily with the elimination target. */
- rtx saved_base_reg = NULL_RTX;
- rtx saved_index_reg = NULL_RTX;
- rtx *base_term = strip_subreg (ad->base_term);
- rtx *index_term = strip_subreg (ad->index_term);
- if (base_term != NULL)
- {
- saved_base_reg = *base_term;
- lra_eliminate_reg_if_possible (base_term);
- if (ad->base_term2 != NULL)
- *ad->base_term2 = *ad->base_term;
- }
- if (index_term != NULL)
- {
- saved_index_reg = *index_term;
- lra_eliminate_reg_if_possible (index_term);
- }
- bool ok_p = valid_address_p (ad->mode, *ad->outer, ad->as);
- if (saved_base_reg != NULL_RTX)
- {
- *base_term = saved_base_reg;
- if (ad->base_term2 != NULL)
- *ad->base_term2 = *ad->base_term;
- }
- if (saved_index_reg != NULL_RTX)
- *index_term = saved_index_reg;
- return ok_p;
-}
-
/* Make reload base reg + disp from address AD. Return the new pseudo. */
static rtx
base_plus_disp_to_reg (struct address_info *ad)
@@ -2832,7 +2866,7 @@ process_address (int nop, rtx *before, r
EXTRA_CONSTRAINT_STR for the validation. */
if (constraint[0] != 'p'
&& EXTRA_ADDRESS_CONSTRAINT (constraint[0], constraint)
- && EXTRA_CONSTRAINT_STR (op, constraint[0], constraint))
+ && valid_address_p (&ad, constraint))
return change_p;
#endif
@@ -3539,7 +3573,7 @@ curr_insn_transform (void)
break;
#ifdef EXTRA_CONSTRAINT_STR
if (EXTRA_MEMORY_CONSTRAINT (c, constraint)
- && EXTRA_CONSTRAINT_STR (tem, c, constraint))
+ && satisfies_memory_constraint_p (tem, constraint))
break;
#endif
}
