Allow subregs around constant displacements [PR116516]

This patch fixes a regression introduced by g:708ee71808ea61758e73.
x86_64 allows addresses of the form:

  (zero_extend:DI (subreg:SI (symbol_ref:DI "foo") 0))

Before the previous patch, a lax SUBREG check meant that we would
treat the subreg as a base and reload it into a base register.
But that wasn't what the target was expecting.  Instead we should
treat "foo" as a constant displacement, to match:

	leal foo, <dest>

After the patch, we recognised that "foo" isn't a base register,
but ICEd on it rather than handling it as a displacement.

With or without the recent patches, if the address had instead been:

  (zero_extend:DI
    (subreg:SI (plus:DI (reg:DI R) (symbol_ref:DI "foo") 0)))

then we would have treated "foo" as the displacement and R as the base
or index, as expected.  The problem was that the code that does this was
rejecting all subregs of objects, rather than just subregs of variable
objects.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


gcc/
	PR middle-end/116516
	* rtlanal.cc (strip_address_mutations): Allow subregs around
	constant displacements.

gcc/testsuite/
	PR middle-end/116516
	* gcc.c-torture/compile/pr116516.c: New test.
---
 gcc/rtlanal.cc                                | 28 ++++++++++++++++---
 .../gcc.c-torture/compile/pr116516.c          | 10 +++++++
 2 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr116516.c

On Thu, Aug 29, 2024 at 8:51 AM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> This patch fixes a regression introduced by g:708ee71808ea61758e73.
> x86_64 allows addresses of the form:
>
>   (zero_extend:DI (subreg:SI (symbol_ref:DI "foo") 0))
>
> Before the previous patch, a lax SUBREG check meant that we would
> treat the subreg as a base and reload it into a base register.
> But that wasn't what the target was expecting.  Instead we should
> treat "foo" as a constant displacement, to match:
>
>         leal foo, <dest>
>
> After the patch, we recognised that "foo" isn't a base register,
> but ICEd on it rather than handling it as a displacement.
>
> With or without the recent patches, if the address had instead been:
>
>   (zero_extend:DI
>     (subreg:SI (plus:DI (reg:DI R) (symbol_ref:DI "foo") 0)))
>
> then we would have treated "foo" as the displacement and R as the base
> or index, as expected.  The problem was that the code that does this was
> rejecting all subregs of objects, rather than just subregs of variable
> objects.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

OK.

Thanks,
Richard.

> Richard
>
>
> gcc/
>         PR middle-end/116516
>         * rtlanal.cc (strip_address_mutations): Allow subregs around
>         constant displacements.
>
> gcc/testsuite/
>         PR middle-end/116516
>         * gcc.c-torture/compile/pr116516.c: New test.
> ---
>  gcc/rtlanal.cc                                | 28 ++++++++++++++++---
>  .../gcc.c-torture/compile/pr116516.c          | 10 +++++++
>  2 files changed, 34 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr116516.c
>
> diff --git a/gcc/rtlanal.cc b/gcc/rtlanal.cc
> index 8afbb32f220..cb0c0c0d719 100644
> --- a/gcc/rtlanal.cc
> +++ b/gcc/rtlanal.cc
> @@ -6467,10 +6467,30 @@ strip_address_mutations (rtx *loc, enum rtx_code *outer_code)
>         /* (and ... (const_int -X)) is used to align to X bytes.  */
>         loc = &XEXP (*loc, 0);
>        else if (code == SUBREG
> -               && !OBJECT_P (SUBREG_REG (*loc))
> -               && subreg_lowpart_p (*loc))
> -       /* (subreg (operator ...) ...) inside and is used for mode
> -          conversion too.  */
> +              && (!OBJECT_P (SUBREG_REG (*loc))
> +                  || CONSTANT_P (SUBREG_REG (*loc)))
> +              && subreg_lowpart_p (*loc))
> +       /* (subreg (operator ...) ...) inside AND is used for mode
> +          conversion too.  It is also used for load-address operations
> +          in which an extension can be done for free, such as:
> +
> +            (zero_extend:DI
> +              (subreg:SI (plus:DI (reg:DI R) (symbol_ref:DI "foo") 0)))
> +
> +          The latter usage also covers subregs of plain "displacements",
> +          such as:
> +
> +            (zero_extend:DI (subreg:SI (symbol_ref:DI "foo") 0))
> +
> +          The inner address should then be the symbol_ref, not the subreg,
> +          similarly to the plus case above.
> +
> +          In contrast, the subreg in:
> +
> +            (zero_extend:DI (subreg:SI (reg:DI R) 0))
> +
> +          should be treated as the base, since it should be replaced by
> +          an SImode hard register during register allocation.  */
>         loc = &SUBREG_REG (*loc);
>        else
>         return loc;
> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr116516.c b/gcc/testsuite/gcc.c-torture/compile/pr116516.c
> new file mode 100644
> index 00000000000..c423ebfef5c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr116516.c
> @@ -0,0 +1,10 @@
> +extern void my_func (int);
> +typedef struct {
> +  int var;
> +} info_t;
> +extern void *_data_offs;
> +void test()
> +{
> +  info_t *info = (info_t *) ((void *)((void *)1) + ((unsigned int)&_data_offs));
> +  my_func(info->var == 0);
> +}
> --
> 2.25.1
>