* [PATCH] rtlanal: Don't assume that calls write to a global SP [PR99596]
@ 2021-04-16 9:17 Richard Sandiford
2021-04-16 10:13 ` Jakub Jelinek
2021-04-16 10:33 ` Richard Biener
0 siblings, 2 replies; 3+ messages in thread
From: Richard Sandiford @ 2021-04-16 9:17 UTC (permalink / raw)
To: gcc-patches
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?
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);
+ }
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] rtlanal: Don't assume that calls write to a global SP [PR99596]
2021-04-16 9:17 [PATCH] rtlanal: Don't assume that calls write to a global SP [PR99596] Richard Sandiford
@ 2021-04-16 10:13 ` Jakub Jelinek
2021-04-16 10:33 ` Richard Biener
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2021-04-16 10:13 UTC (permalink / raw)
To: gcc-patches, richard.sandiford
On Fri, Apr 16, 2021 at 10:17:10AM +0100, Richard Sandiford via Gcc-patches wrote:
> 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.
LGTM, thanks.
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] rtlanal: Don't assume that calls write to a global SP [PR99596]
2021-04-16 9:17 [PATCH] rtlanal: Don't assume that calls write to a global SP [PR99596] Richard Sandiford
2021-04-16 10:13 ` Jakub Jelinek
@ 2021-04-16 10:33 ` Richard Biener
1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2021-04-16 10:33 UTC (permalink / raw)
To: Richard Sandiford, GCC Patches
On Fri, Apr 16, 2021 at 12:01 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> 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);
> + }
> +}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-16 10:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16 9:17 [PATCH] rtlanal: Don't assume that calls write to a global SP [PR99596] Richard Sandiford
2021-04-16 10:13 ` Jakub Jelinek
2021-04-16 10:33 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).