On Fri, Apr 16, 2021 at 12:01 PM Richard Sandiford via Gcc-patches
<[email protected]> wrote:
>
> This patch is a GCC 11 regression caused by the rtl-ssa code.
> Normally we treat calls as containing a potential set of a global
> register, but DF makes a sensible exception for the stack pointer:
>
> if (i == STACK_POINTER_REGNUM)
> /* The stack ptr is used (honorarily) by a CALL insn. */
> df_ref_record (DF_REF_BASE, collection_rec, regno_reg_rtx[i],
> NULL, bb, insn_info, DF_REF_REG_USE,
> DF_REF_CALL_STACK_USAGE | flags);
> else if (global_regs[i])
> {
> /* Calls to const functions cannot access any global registers and
> calls to pure functions cannot set them. All other calls may
> reference any of the global registers, so they are recorded as
> used. */
>
> The only DF definition of SP was therefore the one in the entry block.
> However, the rtlanal.c rtx_properties code (wrongly) assumed that calls
> also clobbered the global SP. This led to multiple definitions of SP
> when we only expected one.
>
> This patch tightens the rtlanal.c handling of global registers
> to match the DF approach.
>
> Tested on aarch64-linux-gnu, arm-linux-gnueabihf, armeb-eabi and
> x86_64-linux-gnu. OK to install?
Makes sense for the stack pointer handling and also for the const/pure
call adjustment. I assume the latter wasn't necessary to fix the testcase
though?
Still OK.
Thanks,
Richard.
> Richard
>
>
> gcc/
> PR rtl-optimization/99596
> * rtlanal.c (rtx_properties::try_to_add_insn): Don't add global
> register accesses for const calls. Assume that pure functions
> can only read from global registers. Ignore cases in which
> the stack pointer has been marked global.
>
> gcc/testsuite/
> PR rtl-optimization/99596
> * gcc.target/arm/pr99596.c: New test.
> ---
> gcc/rtlanal.c | 20 +++++++++++++++-----
> gcc/testsuite/gcc.target/arm/pr99596.c | 18 ++++++++++++++++++
> 2 files changed, 33 insertions(+), 5 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/arm/pr99596.c
>
> diff --git a/gcc/rtlanal.c b/gcc/rtlanal.c
> index c35386bccbd..170420a610b 100644
> --- a/gcc/rtlanal.c
> +++ b/gcc/rtlanal.c
> @@ -2311,15 +2311,25 @@ rtx_properties::try_to_add_insn (const rtx_insn
> *insn, bool include_notes)
> {
> if (CALL_P (insn))
> {
> - /* Adding the global registers first removes a situation in which
> + /* Non-const functions can read from global registers. Impure
> + functions can also set them.
> +
> + Adding the global registers first removes a situation in which
> a fixed-form clobber of register R could come before a real set
> of register R. */
> - if (!hard_reg_set_empty_p (global_reg_set))
> + if (!hard_reg_set_empty_p (global_reg_set)
> + && !RTL_CONST_CALL_P (insn))
> {
> - unsigned int flags = (rtx_obj_flags::IS_READ
> - | rtx_obj_flags::IS_WRITE);
> + unsigned int flags = rtx_obj_flags::IS_READ;
> + if (!RTL_PURE_CALL_P (insn))
> + flags |= rtx_obj_flags::IS_WRITE;
> for (unsigned int regno = 0; regno < FIRST_PSEUDO_REGISTER; ++regno)
> - if (global_regs[regno] && ref_iter != ref_end)
> + /* As a special case, the stack pointer is invariant across calls
> + even if it has been marked global; see the corresponding
> + handling in df_get_call_refs. */
> + if (regno != STACK_POINTER_REGNUM
> + && global_regs[regno]
> + && ref_iter != ref_end)
> *ref_iter++ = rtx_obj_reference (regno, flags,
> reg_raw_mode[regno], 0);
> }
> diff --git a/gcc/testsuite/gcc.target/arm/pr99596.c
> b/gcc/testsuite/gcc.target/arm/pr99596.c
> new file mode 100644
> index 00000000000..2b8b4c87e96
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pr99596.c
> @@ -0,0 +1,18 @@
> +/* { dg-options "-Os -mtune=xscale" } */
> +
> +register int a asm("sp");
> +extern int b;
> +typedef struct {
> + long c[16 * 8 / 32];
> +} d;
> +int e;
> +int f;
> +int g;
> +d h;
> +int j(int, int, int, d);
> +int i(void) {
> + for (;;) {
> + b &&j(e, f, g, h);
> + j(e, f, g, h);
> + }
> +}
