[PATCH] s390: Check for ADDR_REGS in s390_decompose_addrstyle_without_index

[PATCH] s390: Check for ADDR_REGS in s390_decompose_addrstyle_without_index

[Stefan Schulze Frielinghaus]

An explicit check for address registers was not required so far since
during register allocation the processing of address constraints was
sufficient.  However, address constraints themself do not check for
REGNO_OK_FOR_{BASE,INDEX}_P.  Thus, with the newly introduced
late-combine pass in r15-1579-g792f97b44ffc5e we generate new insns with
invalid address registers which aren't fixed up afterwards.

Fixed by explicitly checking for address registers in
s390_decompose_addrstyle_without_index such that those new insns are
rejected.

gcc/ChangeLog:

	target/PR115634
	* config/s390/s390.cc (s390_decompose_addrstyle_without_index):
	Check for ADDR_REGS in s390_decompose_addrstyle_without_index.
---
 This restores bootstrap on s390.  I ran the testsuite against mainline
 and of course there is some fallout which is most likely coming from
 the new pass or other changes.  I have another job running comparing
 pre r15-1579-g792f97b44ffc5e with and without this patch.  Assuming
 this goes well, ok for mainline?

 gcc/config/s390/s390.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
index c65421de831..05a0fde7fb0 100644
--- a/gcc/config/s390/s390.cc
+++ b/gcc/config/s390/s390.cc
@@ -3347,7 +3347,9 @@ s390_decompose_addrstyle_without_index (rtx op, rtx *base,
   while (op && GET_CODE (op) == SUBREG)
     op = SUBREG_REG (op);
 
-  if (op && GET_CODE (op) != REG)
+  if (op && (!REG_P (op)
+	     || (reload_completed
+		 && !REGNO_OK_FOR_BASE_P (REGNO (op)))))
     return false;
 
   if (offset)
-- 
2.45.1

Re: [PATCH] s390: Check for ADDR_REGS in s390_decompose_addrstyle_without_index

[Richard Sandiford]

Stefan Schulze Frielinghaus <stefansf@gcc.gnu.org> writes:
> An explicit check for address registers was not required so far since
> during register allocation the processing of address constraints was
> sufficient.  However, address constraints themself do not check for
> REGNO_OK_FOR_{BASE,INDEX}_P.  Thus, with the newly introduced
> late-combine pass in r15-1579-g792f97b44ffc5e we generate new insns with
> invalid address registers which aren't fixed up afterwards.
>
> Fixed by explicitly checking for address registers in
> s390_decompose_addrstyle_without_index such that those new insns are
> rejected.

Thanks for fixing this.  LGTM FWIW.

Richard

> gcc/ChangeLog:
>
> 	target/PR115634
> 	* config/s390/s390.cc (s390_decompose_addrstyle_without_index):
> 	Check for ADDR_REGS in s390_decompose_addrstyle_without_index.
> ---
>  This restores bootstrap on s390.  I ran the testsuite against mainline
>  and of course there is some fallout which is most likely coming from
>  the new pass or other changes.  I have another job running comparing
>  pre r15-1579-g792f97b44ffc5e with and without this patch.  Assuming
>  this goes well, ok for mainline?
>
>  gcc/config/s390/s390.cc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
> index c65421de831..05a0fde7fb0 100644
> --- a/gcc/config/s390/s390.cc
> +++ b/gcc/config/s390/s390.cc
> @@ -3347,7 +3347,9 @@ s390_decompose_addrstyle_without_index (rtx op, rtx *base,
>    while (op && GET_CODE (op) == SUBREG)
>      op = SUBREG_REG (op);
>  
> -  if (op && GET_CODE (op) != REG)
> +  if (op && (!REG_P (op)
> +	     || (reload_completed
> +		 && !REGNO_OK_FOR_BASE_P (REGNO (op)))))
>      return false;
>  
>    if (offset)

Re: [PATCH] s390: Check for ADDR_REGS in s390_decompose_addrstyle_without_index

[Stefan Schulze Frielinghaus]

On Wed, Jun 26, 2024 at 02:15:18PM +0200, Stefan Schulze Frielinghaus wrote:
> An explicit check for address registers was not required so far since
> during register allocation the processing of address constraints was
> sufficient.  However, address constraints themself do not check for
> REGNO_OK_FOR_{BASE,INDEX}_P.  Thus, with the newly introduced
> late-combine pass in r15-1579-g792f97b44ffc5e we generate new insns with
> invalid address registers which aren't fixed up afterwards.
> 
> Fixed by explicitly checking for address registers in
> s390_decompose_addrstyle_without_index such that those new insns are
> rejected.
> 
> gcc/ChangeLog:
> 
> 	target/PR115634
> 	* config/s390/s390.cc (s390_decompose_addrstyle_without_index):
> 	Check for ADDR_REGS in s390_decompose_addrstyle_without_index.
> ---
>  This restores bootstrap on s390.  I ran the testsuite against mainline
>  and of course there is some fallout which is most likely coming from
>  the new pass or other changes.  I have another job running comparing
>  pre r15-1579-g792f97b44ffc5e with and without this patch.  Assuming
>  this goes well, ok for mainline?

Bootstrap and regtest of this test went also fine.

> 
>  gcc/config/s390/s390.cc | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc
> index c65421de831..05a0fde7fb0 100644
> --- a/gcc/config/s390/s390.cc
> +++ b/gcc/config/s390/s390.cc
> @@ -3347,7 +3347,9 @@ s390_decompose_addrstyle_without_index (rtx op, rtx *base,
>    while (op && GET_CODE (op) == SUBREG)
>      op = SUBREG_REG (op);
>  
> -  if (op && GET_CODE (op) != REG)
> +  if (op && (!REG_P (op)
> +	     || (reload_completed
> +		 && !REGNO_OK_FOR_BASE_P (REGNO (op)))))
>      return false;
>  
>    if (offset)
> -- 
> 2.45.1
> 

Re: [PATCH] s390: Check for ADDR_REGS in s390_decompose_addrstyle_without_index

[Andreas Krebbel]

On 6/26/24 14:15, Stefan Schulze Frielinghaus wrote:
> An explicit check for address registers was not required so far since
> during register allocation the processing of address constraints was
> sufficient.  However, address constraints themself do not check for
> REGNO_OK_FOR_{BASE,INDEX}_P.  Thus, with the newly introduced
> late-combine pass in r15-1579-g792f97b44ffc5e we generate new insns with
> invalid address registers which aren't fixed up afterwards.
>
> Fixed by explicitly checking for address registers in
> s390_decompose_addrstyle_without_index such that those new insns are
> rejected.
>
> gcc/ChangeLog:
>
> 	target/PR115634
> 	* config/s390/s390.cc (s390_decompose_addrstyle_without_index):
> 	Check for ADDR_REGS in s390_decompose_addrstyle_without_index.

Ok. Thanks!


Andreas