This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, RFC] LRA subreg handling


Jeff Law <law@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.

Thanks,
Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]