Re: [Patch, lra] PR70751, correct the cost for spilling non-pseudo into memory

[Jiong Wang <jiong dot wang at foss dot arm dot com>]

Andreas Krebbel writes:

> On 06/28/2016 04:16 PM, Jiong Wang wrote:
> ...
>> So my first impression is TARGET_LEGITIMATE_ADDRESS_P on s390 do need a
>> fix here. The following draft patch fix this, my fix may be in
>> correct as normally we will allow illegal constant offset if it's
>> associated with virtual frame register before virtual register
>> elimination, I don't know if that should be considered here. Also I
>> don't know if such check should be constrainted under some architecture
>> features.
>> 
>> --- a/gcc/config/s390/s390.c
>> +++ b/gcc/config/s390/s390.c
>> @@ -4374,6 +4374,11 @@ s390_legitimate_address_p (machine_mode mode, rtx 
>> addr, bool strict)
>>                 || REGNO_REG_CLASS (REGNO (ad.indx)) == ADDR_REGS))
>>            return false;
>>       }
>> +
>> +  if (ad.disp
>> +      && !s390_short_displacement (ad.disp))
>> +    return false;
>> +
>>     return true;
>>   }
>
> Whether short displacement is required or not depends on the instruction. We always relied on reload
> to take care of this.  legitimate_address_p needs to accept the union of all valid adresses on
> S/390.  That change probably would cause a severe performance regression.
>
>> Meanwhile I feel LRA might be too strict here, as we can't assume all
>> address legitimized before lra, right? we still need to handle reload
>> illegal address. While now, a reload of
>> 
>>    set (mem (reg + reg + offset)) regA
>> 
>>    (The offset is out of range that the inner address doesn't pass
>>     constraint check.)
>> 
>> into
>> 
>>     regB <- reg + reg + offset
>>    (set (mem (regB)) regA)
>
> This is exactly what is supposed to happen in that case. All addresses which might be invalid to be
> used directly in an insn can be handled with load address or load address relative long (+ secondary
> reload for odd addresses).
>
>> is treate as spill after r237277, so I feel the following check in
>> lra-constraints.c also needs a relax?
>> 
>> if (no_regs_p && !(REG_P (op) && hard_regno[nop] < 0))
>> 
>> 
>> Comments?

I am testing a fix which teach LRA don't add spill cost if it's "offmemok".

-- 
Regards,
Jiong