Hi,

The variables first_moveable_pseudo and last_moveable_pseudo aren't reset
after compiling a function, which means they leak into the first scheduler
pass of the following function. In some cases, this can cause an extra spill
during register allocation of the second function.

This patch fixes this by setting

                first_moveable_pseudo = last_moveable_pseudo

at the beginning of the first scheduler pass.

Because the spill occurs in the middle of the IRA pass it is highly
target-sensitive, so I have provided a test case that works on aarch64. There
doesn't seem to be a target-independent way to trigger this bug, since the
RTL at the IRA stage is already quite target-specific.

Interestingly, the aarch64 test case here doesn't actually perform a complete
register spill - the value is stored on the stack but never retrieved.
Perhaps this points to another, deeper bug?

Bootstrapped and regression tested on aarch64-none-linux-gnu.

I don't have write privileges, so if it's fine could someone push for me?

Thanks,
Omar

----

gcc/ChangeLog:

2020-06-30: Omar Tahir <omar.ta...@arm.com>

                * sched-rgn.c: Include ira-int.h, ira.h, regs.h.
                (rest_of_handle_sched): Clear moveable pseudo range.

gcc/testsuite/ChangeLog:

2020-06-30: Omar Tahir <omar.ta...@arm.com>

                * gcc.target/aarch64/nospill.c: New test.

----

diff --git a/gcc/sched-rgn.c b/gcc/sched-rgn.c
index 7f5dfdb..51329fc 100644
--- a/gcc/sched-rgn.c
+++ b/gcc/sched-rgn.c
@@ -65,6 +65,9 @@ along with GCC; see the file COPYING3.  If not see
#include "dbgcnt.h"
#include "pretty-print.h"
#include "print-rtl.h"
+#include "ira.h"
+#include "regs.h"
+#include "ira-int.h"

/* Disable warnings about quoting issues in the pp_xxx calls below
    that (intentionally) don't follow GCC diagnostic conventions.  */
@@ -3719,6 +3722,7 @@ static unsigned int
rest_of_handle_sched (void)
{
#ifdef INSN_SCHEDULING
+  first_moveable_pseudo = last_moveable_pseudo;
   if (flag_selective_scheduling
       && ! maybe_skip_selective_scheduling ())
     run_selective_scheduling ();
diff --git a/gcc/testsuite/gcc.target/aarch64/nospill.c 
b/gcc/testsuite/gcc.target/aarch64/nospill.c
new file mode 100644
index 0000000..60399d8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/nospill.c
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+/* The pseudo for P is marked as moveable in the IRA pass. */
+float
+func_0 (float a, float b, float c)
+{
+  float p = c / a;
+
+  if (b > 1)
+    {
+      b /= p;
+      if (c > 2)
+        a /= 3;
+    }
+
+  return b / c * a;
+}
+
+/* If first_moveable_pseudo and last_moveable_pseudo are not reset correctly,
+   they will carry over and spill the pseudo for Q. */
+float
+func_1 (float a, float b, float c)
+{
+  float q = a + b;
+
+  c *= a / (b + b);
+  if (a > 0)
+    c *= q;
+
+  return a * b * c;
+}
+
+/* We have plenty of spare registers, so check nothing has been spilled. */
+/* { dg-final { scan-assembler-not "str" } } */

Attachment: moveable_pseudo.patch
Description: moveable_pseudo.patch

Reply via email to