Message ID | mpt1qfjz13s.fsf@arm.com |
---|---|
State | New |
Headers | show |
Series | lra: Avoid unfolded plus-0 | expand |
On 8/31/23 09:24, Richard Sandiford via Gcc-patches wrote: > While backporting another patch to an earlier release, I hit a > situation in which lra_eliminate_regs_1 would eliminate an address to: > > (plus (reg:P R) (const_int 0)) > > This address compared not-equal to plain: > > (reg:P R) > > which caused an ICE in a later peephole2. (The ICE showed up in > gfortran.fortran-torture/compile/pr80464.f90 on the branch but seems > to be latent on trunk.) > > These unfolded PLUSes shouldn't occur in the insn stream, and later code > in the same function tried to avoid them. > > Tested on aarch64-linux-gnu so far, but I'll test on x86_64-linux-gnu too. > Does this look OK? > > There are probably other instances of the same thing elsewhere, > but it seemed safer to stick to the one that caused the issue. > > Thanks, > Richard > > > gcc/ > * lra-eliminations.cc (lra_eliminate_regs_1): Use simplify_gen_binary > rather than gen_rtx_PLUS. OK jeff
Vlad, is it OK if I backport the patch below to fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111528 ? Jakub has given a conditional OK on irc. Thanks, Richard Richard Sandiford <richard.sandiford@arm.com> writes: > While backporting another patch to an earlier release, I hit a > situation in which lra_eliminate_regs_1 would eliminate an address to: > > (plus (reg:P R) (const_int 0)) > > This address compared not-equal to plain: > > (reg:P R) > > which caused an ICE in a later peephole2. (The ICE showed up in > gfortran.fortran-torture/compile/pr80464.f90 on the branch but seems > to be latent on trunk.) > > These unfolded PLUSes shouldn't occur in the insn stream, and later code > in the same function tried to avoid them. > > Tested on aarch64-linux-gnu so far, but I'll test on x86_64-linux-gnu too. > Does this look OK? > > There are probably other instances of the same thing elsewhere, > but it seemed safer to stick to the one that caused the issue. > > Thanks, > Richard > > > gcc/ > * lra-eliminations.cc (lra_eliminate_regs_1): Use simplify_gen_binary > rather than gen_rtx_PLUS. > --- > gcc/lra-eliminations.cc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/lra-eliminations.cc b/gcc/lra-eliminations.cc > index df613cdda76..4daaff1a124 100644 > --- a/gcc/lra-eliminations.cc > +++ b/gcc/lra-eliminations.cc > @@ -406,7 +406,7 @@ lra_eliminate_regs_1 (rtx_insn *insn, rtx x, machine_mode mem_mode, > elimination_fp2sp_occured_p = true; > > if (! update_p && ! full_p) > - return gen_rtx_PLUS (Pmode, to, XEXP (x, 1)); > + return simplify_gen_binary (PLUS, Pmode, to, XEXP (x, 1)); > > if (maybe_ne (update_sp_offset, 0)) > offset = ep->to_rtx == stack_pointer_rtx ? update_sp_offset : 0;
On 10/18/23 09:37, Richard Sandiford wrote: > Vlad, is it OK if I backport the patch below to fix > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111528 ? Jakub has > given a conditional OK on irc. > Ok. It should be safe. I don't expect any issues because of this.
diff --git a/gcc/lra-eliminations.cc b/gcc/lra-eliminations.cc index df613cdda76..4daaff1a124 100644 --- a/gcc/lra-eliminations.cc +++ b/gcc/lra-eliminations.cc @@ -406,7 +406,7 @@ lra_eliminate_regs_1 (rtx_insn *insn, rtx x, machine_mode mem_mode, elimination_fp2sp_occured_p = true; if (! update_p && ! full_p) - return gen_rtx_PLUS (Pmode, to, XEXP (x, 1)); + return simplify_gen_binary (PLUS, Pmode, to, XEXP (x, 1)); if (maybe_ne (update_sp_offset, 0)) offset = ep->to_rtx == stack_pointer_rtx ? update_sp_offset : 0;