This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch, lra] PR70751, correct the cost for spilling non-pseudo into memory
- From: Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>
- To: Jiong Wang <jiong dot wang at foss dot arm dot com>, vogt at linux dot vnet dot ibm dot com, Jeff Law <law at redhat dot com>, Vladimir N Makarov <vmakarov at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Robin Dapp <rdapp at linux dot vnet dot ibm dot com>
- Date: Wed, 29 Jun 2016 09:18:30 +0200
- Subject: Re: [Patch, lra] PR70751, correct the cost for spilling non-pseudo into memory
- Authentication-results: sourceware.org; auth=none
- References: <cfd17ab9-b740-6516-6212-41361177a8a5 at foss dot arm dot com> <0d4316cc-8ee3-b5cc-9c26-50365ff82201 at redhat dot com> <20160627162614 dot GA30612 at linux dot vnet dot ibm dot com> <4f0aadf8-f12e-4ef3-c4d5-c97e31a89b87 at foss dot arm dot com> <bcc5bf63-6d5d-3516-221d-33276fe5ae71 at foss dot arm dot com> <f5afe3a5-aa9b-9aa5-33ef-299298f21a27 at foss dot arm 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?
>
- References:
- [Patch, lra] PR70751, correct the cost for spilling non-pseudo into memory
- Re: [Patch, lra] PR70751, correct the cost for spilling non-pseudo into memory
- Re: [Patch, lra] PR70751, correct the cost for spilling non-pseudo into memory
- Re: [Patch, lra] PR70751, correct the cost for spilling non-pseudo into memory
- Re: [Patch, lra] PR70751, correct the cost for spilling non-pseudo into memory
- Re: [Patch, lra] PR70751, correct the cost for spilling non-pseudo into memory