Richard Sandiford <rdsandif...@googlemail.com> writes:
> Jeff Law <l...@redhat.com> writes:
> > On 01/15/15 03:13, Robert Suchanek wrote:
> >>> Robert, can you look at reload.c::reload_inner_reg_of_subreg and
> >>> verify that the comment just before its return statement is
> >>> effectively the situation you're in.
> >>>
> >>> There are certainly cases where a SUBREG needs to be treated as an
> >>> in-out operand. We walked through them eons ago when we were poking
> >>> at SSA for RTL. But the details have long since faded from memory.
> >>
> >> The comment pretty much applies to my situation. The only difference
> >> I can see is that reload would have had hard registers at this point.
> >> In the testcase, LRA does not have hard registers assigned to the
> >> concerned pseudo(s), thus, it can't rely on the information in
> >> hard_regno_nregs to check if the number of registers in INNER is
> >> different to the number of words in INNER.
>
> But the code you quote is:
>
> if (GET_CODE (*loc) == SUBREG)
> {
> reg = SUBREG_REG (*loc);
> byte = SUBREG_BYTE (*loc);
> if (REG_P (reg)
> /* Strict_low_part requires reload the register not
> the sub-register. */
> && (curr_static_id->operand[i].strict_low
> || (GET_MODE_SIZE (mode)
> <= GET_MODE_SIZE (GET_MODE (reg))
> && (hard_regno
> = get_try_hard_regno (REGNO (reg))) >= 0
> && (simplify_subreg_regno
> (hard_regno,
> GET_MODE (reg), byte, mode) < 0)
> && (goal_alt[i] == NO_REGS
> || (simplify_subreg_regno
> (ira_class_hard_regs[goal_alt[i]][0],
> GET_MODE (reg), byte, mode) >= 0))))
>
> Here we do have a hard register, but it isn't valid to form the subreg
> on that hard register. Reload had to cope with that case too.
>
> Since the subreg on the original hard register is invalid, we can't use
> it to decide whether the intention was to write to only a part of the
> inner register. But I don't think we need to use the hard register
> here. The original register was a psuedo and I'm pretty sure...
>
> > The differences (hard vs pseudo regs) are primarily an implementation
> > detail. I was really looking to see if there was existing code which
> > would turn an output reload into an in-out reload for these subregs.
> >
> > The in-out nature of certain subregs is something I've personally
> > stumbled over in various contexts (for example, this also came up
> > during RTL-SSA investigations years ago).
>
> ...the rule for pseudos is that words of a multiword pseudo can be
> accessed independently but subword pieces of an individual word can't.
> This obviously isn't ideal if a mode is intended for wider-than-word
> registers, since not all words will be independently addressable when
> allocated to those registers. The code above is partly dealing with the
> fallout from that. It's also why we have strict_lowpart for cases like
> al on i386.
>
> So IMO the patch is too broad. I think it should only use INOUT reloads
> for !strict_low if the inner mode is wider than a word and the outer
> mode is strictly narrower than the inner mode. That's on top of Vlad's
> comment about only converting OP_OUTs, of course.
This sounds correct/sensible. The change as committed will be turning
some OP_OUT reloads into OP_INOUT unnecessarily. Checking for !strict_low
is however probably redundant as strict_low must be OP_INOUT and never
OP_OUT but we could mask backend bugs by converting strict_low subregs.
I think this should be resolved for GCC 5 if others agree it is an issue.
Matthew
