diff mbox series

lra: Avoid unfolded plus-0

Message ID mpt1qfjz13s.fsf@arm.com
State New
Headers show
Series lra: Avoid unfolded plus-0 | expand

Commit Message

Richard Sandiford Aug. 31, 2023, 3:24 p.m. UTC
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(-)

Comments

Jeff Law Sept. 5, 2023, 5:05 a.m. UTC | #1
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
Richard Sandiford Oct. 18, 2023, 1:37 p.m. UTC | #2
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;
Vladimir Makarov Oct. 18, 2023, 1:59 p.m. UTC | #3
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 mbox series

Patch

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;