| Message ID | CAMqJFCrZwam03uO1jAVQ_Wck-0VgJFKFZQ-jz7-+NbxrOEksqQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | RFA: avoid infinite lra loop for constant addresses | expand |
On 5/18/2021 2:56 AM, Joern Rennecke wrote: > I find that when compiling some files, lra goes into an infinite loop > reloading constant > addresses. This patch allows them to just be recognized as matching addresses > immediately, which also saves a bit of space for a few other files. > > Bootstrapped and regression tested on x86_64-pc-linux-gnu. > > process_addr_reg-elim.txt > > gcc/ > * lra-constraints.c: New arguments mem_mode and as. Changed caller. > If equivalence search has yielded a constant that is valid as an > address, use it. At first glance it seems to me like you're papering over a target bug. Vlad's got the final call though. jeff
On Tue, May 18, 2021 at 11:52:27AM -0600, Jeff Law via Gcc-patches wrote: > On 5/18/2021 2:56 AM, Joern Rennecke wrote: > >I find that when compiling some files, lra goes into an infinite loop > >reloading constant > >addresses. This patch allows them to just be recognized as matching > >addresses > >immediately, which also saves a bit of space for a few other files. > > > >Bootstrapped and regression tested on x86_64-pc-linux-gnu. > > > >process_addr_reg-elim.txt > > > >gcc/ > > * lra-constraints.c: New arguments mem_mode and as. Changed caller. > > If equivalence search has yielded a constant that is valid as an > > address, use it. > > At first glance it seems to me like you're papering over a target bug. Yeah. It means that curr_insn_transform did not return early, which it should if the insn is a single_set (likely, but Joern didn't show the actual insns, so we don't know), or simple_move_p isn't true. And the latter means that register_move_cost isn't 2, which all trivial moves should have. Does that help? Segher
diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c index a766f1fd7e8..8451da58164 100644 --- a/gcc/lra-constraints.c +++ b/gcc/lra-constraints.c @@ -1454,10 +1454,14 @@ static int curr_swapped; the RTL was changed. if CHECK_ONLY_P is true, check that the *LOC is a correct address - register. Return false if the address register is correct. */ + register. Return false if the address register is correct. + + if MEM_MODE is not VOIDmode, then *LOC is the entire address for a + memory access of MODE in address space AS, and *LOC may be replaced + with a constant if that is a valid address. */ static bool process_addr_reg (rtx *loc, bool check_only_p, rtx_insn **before, rtx_insn **after, - enum reg_class cl) + enum reg_class cl, machine_mode mem_mode, addr_space_t as) { int regno; enum reg_class rclass, new_class; @@ -1502,6 +1506,13 @@ process_addr_reg (rtx *loc, bool check_only_p, rtx_insn **before, rtx_insn **aft if (! check_only_p && (*loc = get_equiv_with_elimination (reg, curr_insn)) != reg) { + /* If the elimination has yielded a constant that is fine as an + address, don't try to reload that. */ + if (CONSTANT_P (*loc) && mem_mode != VOIDmode + && strict_memory_address_addr_space_p + (mem_mode, *loc, as)) + return true; + if (lra_dump_file != NULL) { fprintf (lra_dump_file, @@ -3523,7 +3534,12 @@ process_address_1 (int nop, bool check_only_p, REGNO (*ad.base_term)) != NULL_RTX) ? after : NULL), base_reg_class (ad.mode, ad.as, ad.base_outer_code, - get_index_code (&ad))))) + get_index_code (&ad)), + ((MEM_P (mem) && &XEXP (mem, 0) == ad.base_term) + || (SUBREG_P (op) && MEM_P (SUBREG_REG (op)) + && &XEXP (SUBREG_REG (op), 0) == ad.base_term) + ? ad.mode : VOIDmode), + ad.as))) { change_p = true; if (ad.base_term2 != NULL) @@ -3531,7 +3547,7 @@ process_address_1 (int nop, bool check_only_p, } if (ad.index_term != NULL && process_addr_reg (ad.index_term, check_only_p, - before, NULL, INDEX_REG_CLASS)) + before, NULL, INDEX_REG_CLASS, VOIDmode, ad.as)) change_p = true; /* Target hooks sometimes don't treat extra-constraint addresses as