On May 30, 2014, at 10:39 AM, Jeff Law <l...@redhat.com> 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.