This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[lra] Cleanup the use of offmemok and don't count spilling cost for it
- From: Jiong Wang <jiong dot wang at foss dot arm dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Andreas Krebbel <krebbel at linux dot vnet dot ibm 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>, Robin Dapp <rdapp at linux dot vnet dot ibm dot com>
- Date: Thu, 30 Jun 2016 18:24:02 +0100
- Subject: [lra] Cleanup the use of offmemok and don't count spilling cost for it
- 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> <57737646 dot 30704 at linux dot vnet dot ibm dot com> <n99oa6jisu6 dot fsf at foss dot arm dot com>
On 29/06/16 22:31, Jiong Wang wrote:
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".
Here is the patch,
From my understanding, "offmemok" is used to represent a memory operand
who's address we want to reload, and searching of it's reference location
seems confirmed my understanding as it's always used together with MEM_P
check.
So this patch does the following modifications:
* Only set offmemok to true if MEM_P is also true, as otherwise offmemok
is not used.
* Remove redundant MEM_P check which was used together with offmemok.
* Avoid the addition of spilling cost if offmemok be true as an address
calculation reload is not spilling.
bootstrap & gcc/g++ regression OK on x86_64/aarch64/arm.
OK for trunk?
2016-06-30 Jiong Wang <jiong.wang@arm.com>
gcc
* lra-constraints.c (process_alt_operands): Only set "offmemok" for
MEM_P. Remove redundant MEM_P check if it's used together with
"offmemok" check. Don't add spilling cost for "offmemok".
(curr_insn_transform): Remove redundant MEM_P check.
diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c
index bf08dce..4546f1e 100644
--- a/gcc/lra-constraints.c
+++ b/gcc/lra-constraints.c
@@ -1967,7 +1967,6 @@ process_alt_operands (int only_alternative)
combination, because we can't reload
it. */
if (curr_alt_offmemok[m]
- && MEM_P (*curr_id->operand_loc[m])
&& curr_alt[m] == NO_REGS && ! curr_alt_win[m])
continue;
}
@@ -2089,7 +2088,8 @@ process_alt_operands (int only_alternative)
|| equiv_substition_p[nop])
badop = false;
constmemok = true;
- offmemok = true;
+ if (MEM_P (op))
+ offmemok = true;
break;
case CT_ADDRESS:
@@ -2436,8 +2436,7 @@ process_alt_operands (int only_alternative)
reject += LRA_MAX_REJECT;
}
- if (! (MEM_P (op) && offmemok)
- && ! (const_to_mem && constmemok))
+ if (!offmemok && ! (const_to_mem && constmemok))
{
/* We prefer to reload pseudos over reloading other
things, since such reloads may be able to be
@@ -2488,7 +2487,9 @@ process_alt_operands (int only_alternative)
Code below increases the reject for both pseudo and non-pseudo
spill. */
- if (no_regs_p && !(REG_P (op) && hard_regno[nop] < 0))
+ if (no_regs_p
+ && !offmemok
+ && !(REG_P (op) && hard_regno[nop] < 0))
{
if (lra_dump_file != NULL)
fprintf
@@ -3909,7 +3910,7 @@ curr_insn_transform (bool check_only_p)
appearing where an offsettable address will do. It also may
be a case when the address should be special in other words
not a general one (e.g. it needs no index reg). */
- if (goal_alt_matched[i][0] == -1 && goal_alt_offmemok[i] && MEM_P (op))
+ if (goal_alt_matched[i][0] == -1 && goal_alt_offmemok[i])
{
enum reg_class rclass;
rtx *loc = &XEXP (op, 0);
- 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
- 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