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

[Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>]

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?
>