On May 30, 2014, at 10:39 AM, Jeff Law <[email protected]> wrote:
> On 05/25/14 18:19, Matt Thomas wrote:
>>
>> But even if movhi is a define_expand, as far as I can tell there's
>> isn't enough info to know whether that is possible. At that time,
>> how can I tell that operands[0] will be a hard reg or operands[1]
>> will be subreg of a mode dependent memory access?
> At that time, you can't know those things. Not even close ;-) You certainly
> don't want to try and rewrite the insn to just use SImode. This is all an
> indication something has gone wrong elsewhere and this would just paper over
> the problem.
>
>>
>> I've tried using secondary_reload and it called called with
>>
>> (subreg:HI (reg:SI 113 [ MEM[base: _154, offset: 0B] ]) 0)
>>
>> but it dies in change_address_1 before invoking the code returned in
>> sri.
> I suspect if you dig deep enough, you can make a secondary reload do what you
> want. It's just amazingly painful.
>
> You want to allocate an SImode temporary, do the load of the SI memory
> location into that SImode temporary, then (subreg:SI (tempreg:SI)). Your best
> bet is going to be to look at how some other ports handle their secondary
> reloads. But I warn you, it's going to be painful.
Doesn't work because the assert fires before the secondary reload takes place.
In expr.c:convert_mode there is code that would seem to prevent this:
/* For truncation, usually we can just refer to FROM in a narrower mode. */
if (GET_MODE_BITSIZE (to_mode) < GET_MODE_BITSIZE (from_mode)
&& TRULY_NOOP_TRUNCATION_MODES_P (to_mode, from_mode))
{
if (!((MEM_P (from)
&& ! MEM_VOLATILE_P (from)
&& direct_load[(int) to_mode]
&& ! mode_dependent_address_p (XEXP (from, 0),
MEM_ADDR_SPACE (from)))
|| REG_P (from)
|| GET_CODE (from) == SUBREG))
from = force_reg (from_mode, from);
if (REG_P (from) && REGNO (from) < FIRST_PSEUDO_REGISTER
&& ! HARD_REGNO_MODE_OK (REGNO (from), to_mode))
from = copy_to_reg (from);
emit_move_insn (to, gen_lowpart (to_mode, from));
return;
}
but from at that point is just
(mem:SI (reg:SI 112 [ D.118399 ]) [4 MEM[base: _154, offset: 0B]+0 S4 A32])
So there is not enough information for mode_dependent_address_p to return true.
>>
>> I've tracked this down to reload replacing (reg:SI 113) with
>> reg_equiv_mem (133) in the rtx. However, it doesn't verify the rtx
>> is actually valid. I added a gcc_assert to trap this and got:
> Right. reload will make that replacement and it's not going to do any
> verification at that point. Verification would have happened earlier.
See above. If anywhere, that is where it would have been done.
> You have to look at the beginning of the main reload loop and poke at that
> for a while:
>
> /* For each pseudo register that has an equivalent location defined,
> try to eliminate any eliminable registers (such as the frame pointer)
> assuming initial offsets for the replacement register, which
> is the normal case.
>
> If the resulting location is directly addressable, substitute
> the MEM we just got directly for the old REG.
>
> If it is not addressable but is a constant or the sum of a hard reg
> and constant, it is probably not addressable because the constant is
> out of range, in that case record the address; we will generate
> hairy code to compute the address in a register each time it is
> needed. Similarly if it is a hard register, but one that is not
> valid as an address register.
>
> If the location is not addressable, but does not have one of the
> above forms, assign a stack slot. We have to do this to avoid the
> potential of producing lots of reloads if, e.g., a location involves
> a pseudo that didn't get a hard register and has an equivalent memory
> location that also involves a pseudo that didn't get a hard register.
>
> Perhaps at some point we will improve reload_when_needed handling
> so this problem goes away. But that's very hairy. */
I found a simplier solution. It seemed to me that reload_inner_reg_of_subreg
was the right place to make this happen. The following diff (to gcc 4.8.3)
fixes the problem:
diff -u -p -r1.3 reload.c
--- gcc/reload.c 1 Mar 2014 08:58:29 -0000 1.3
+++ gcc/reload.c 3 Jun 2014 17:24:27 -0000
@@ -846,6 +846,7 @@ static bool
reload_inner_reg_of_subreg (rtx x, enum machine_mode mode, bool output)
{
rtx inner;
+ int regno;
/* Only SUBREGs are problematical. */
if (GET_CODE (x) != SUBREG)
@@ -857,10 +858,20 @@ reload_inner_reg_of_subreg (rtx x, enum
if (CONSTANT_P (inner) || GET_CODE (inner) == PLUS)
return true;
- /* If INNER is not a hard register, then INNER will not need reloading. */
- if (!(REG_P (inner) && HARD_REGISTER_P (inner)))
+ /* If INNER is not a register, then INNER will not need reloading. */
+ if (!REG_P (inner))
return false;
+ regno = REGNO (inner);
+
+ /* If INNER is not a hard register, then INNER will not need reloading
+ unless it's a mode dependent memory reference. */
+ if (regno >= FIRST_PSEUDO_REGISTER)
+ return !output
+ && reg_equiv_mem (regno) != 0
+ && mode_dependent_address_p (XEXP (reg_equiv_mem (regno), 0),
+ MEM_ADDR_SPACE (reg_equiv_mem (regno)));
+
/* If INNER is not ok for MODE, then INNER will need reloading. */
if (!HARD_REGNO_MODE_OK (subreg_regno (x), mode))
return true;
@@ -1142,7 +1153,7 @@ push_reload (rtx in, rtx out, rtx *inloc
if (in != 0 && reload_inner_reg_of_subreg (in, inmode, false))
{
- if (REG_P (SUBREG_REG (in)))
+ if (REG_P (SUBREG_REG (in)) && HARD_REGISTER_P (SUBREG_REG (in)))
subreg_in_class
= find_valid_class (inmode, GET_MODE (SUBREG_REG (in)),
subreg_regno_offset (REGNO (SUBREG_REG (in)),
@@ -1150,7 +1161,8 @@ push_reload (rtx in, rtx out, rtx *inloc
SUBREG_BYTE (in),
GET_MODE (in)),
REGNO (SUBREG_REG (in)));
- else if (GET_CODE (SUBREG_REG (in)) == SYMBOL_REF)
+ else if (REG_P (SUBREG_REG (in))
+ || GET_CODE (SUBREG_REG (in)) == SYMBOL_REF)
subreg_in_class = find_valid_class_1 (inmode,
GET_MODE (SUBREG_REG (in)),
rclass);
Give that reload_inner_reg_of_subreg returns true for CONSTANT_P or PLUS,
I have to think that last else shouldn't have the if (...) and just be
a naked else.
