[PATCH] Allow subregs around constant displacements [PR116516]

[Richard Biener]

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
>