diff mbox series

[2/2v2,ira] avoid resetting ira_reg_equiv for function invariants

Message ID orms6vveu6.fsf_-_@lxoliva.fsfla.org
State Committed
Commit 12992edaa96897d4ce26c3d48fa19c142b0aef28
Headers
Series [2/2v2,ira] avoid resetting ira_reg_equiv for function invariants |

Commit Message

Alexandre Oliva Sept. 15, 2025, 4:51 p.m. UTC 
  An aarch64 toolchain built with --enable-default-pie fails
gcc.target/aarch64/sme/nonlocal_goto_[123].c because the register
allocator ends up resetting equivalences, so it concludes it needs to
preserve a rematerializable function invariant across a call instead
of rematerializing it.

Restore the find_reg_equiv_invariant_const logic that was in place
before commit 55a2c3226a3e90a6d65f19710bab1ac377054234 rewrote it and
added a (spurious?) requirement for non-function-invariants to retain
an equivalence when other PIC-related conditions didn't apply.  That
requirement seems either reversed or unnecessary.

Regstrapped on x86_64-linux-gnu, with and without --enable-host-pie, and
on aarch64-linux-gnu, with --enable-host-pie --enable-default-pie.  This
version of the patch even makes sense to me! :-) Ok to install?


for  gcc/ChangeLog

	* ira.cc (setup_reg_equiv): Retain function invariant
	equivalences.
---
 gcc/ira.cc |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Vladimir Makarov Sept. 18, 2025, 8:14 p.m. UTC | #1 
On 9/15/25 12:51 PM, Alexandre Oliva wrote:
> An aarch64 toolchain built with --enable-default-pie fails
> gcc.target/aarch64/sme/nonlocal_goto_[123].c because the register
> allocator ends up resetting equivalences, so it concludes it needs to
> preserve a rematerializable function invariant across a call instead
> of rematerializing it.
>
> Restore the find_reg_equiv_invariant_const logic that was in place
> before commit 55a2c3226a3e90a6d65f19710bab1ac377054234 rewrote it and
> added a (spurious?) requirement for non-function-invariants to retain
> an equivalence when other PIC-related conditions didn't apply.  That
> requirement seems either reversed or unnecessary.
>
> Regstrapped on x86_64-linux-gnu, with and without --enable-host-pie, and
> on aarch64-linux-gnu, with --enable-host-pie --enable-default-pie.  This
> version of the patch even makes sense to me! :-) Ok to install?
>
Yes, I guess it is my 13 years old mistake.  The code you are proposing 
is the same existed in the old reload pass.

The patch is ok for me. Than you for fixing this up, Alex.


> for  gcc/ChangeLog
>
> 	* ira.cc (setup_reg_equiv): Retain function invariant
> 	equivalences.
> ---
>   gcc/ira.cc |    6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/ira.cc b/gcc/ira.cc
> index 4eebc9c4c50fb..3e15966558edf 100644
> --- a/gcc/ira.cc
> +++ b/gcc/ira.cc
> @@ -4233,12 +4233,14 @@ setup_reg_equiv (void)
>   			    && REGNO (SET_SRC (set)) == (unsigned int) i);
>   		x = SET_DEST (set);
>   	      }
> -	    if (! function_invariant_p (x)
> +	    /* If PIC is enabled and the equiv is not a LEGITIMATE_PIC_OPERAND,
> +	       we can't use it.  */
> +	    if (! CONSTANT_P (x)
>   		|| ! flag_pic
>   		/* A function invariant is often CONSTANT_P but may
>   		   include a register.  We promise to only pass
>   		   CONSTANT_P objects to LEGITIMATE_PIC_OPERAND_P.  */
> -		|| (CONSTANT_P (x) && LEGITIMATE_PIC_OPERAND_P (x)))
> +		|| LEGITIMATE_PIC_OPERAND_P (x))
>   	      {
>   		/* It can happen that a REG_EQUIV note contains a MEM
>   		   that is not a legitimate memory operand.  As later
>
diff mbox series

Patch

diff --git a/gcc/ira.cc b/gcc/ira.cc
index 4eebc9c4c50fb..3e15966558edf 100644
--- a/gcc/ira.cc
+++ b/gcc/ira.cc
@@ -4233,12 +4233,14 @@  setup_reg_equiv (void)
 			    && REGNO (SET_SRC (set)) == (unsigned int) i);
 		x = SET_DEST (set);
 	      }
-	    if (! function_invariant_p (x)
+	    /* If PIC is enabled and the equiv is not a LEGITIMATE_PIC_OPERAND,
+	       we can't use it.  */
+	    if (! CONSTANT_P (x)
 		|| ! flag_pic
 		/* A function invariant is often CONSTANT_P but may
 		   include a register.  We promise to only pass
 		   CONSTANT_P objects to LEGITIMATE_PIC_OPERAND_P.  */
-		|| (CONSTANT_P (x) && LEGITIMATE_PIC_OPERAND_P (x)))
+		|| LEGITIMATE_PIC_OPERAND_P (x))
 	      {
 		/* It can happen that a REG_EQUIV note contains a MEM
 		   that is not a legitimate memory operand.  As later