On Thu, Aug 29, 2024 at 8:51 AM Richard Sandiford
<richard.sandif...@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
>
