[PATCH] sched-deps: do not schedule pseudos across calls [PR108117]

[Alexander Monakov]

On Fri, 23 Dec 2022, Jose E. Marchesi wrote:

> > +1 for trying this FWIW.  There's still plenty of time to try an
> > alternative solution if there are unexpected performance problems.
> 
> Let me see if Alexander's patch fixes the issue at hand (it must) and
> will also do some regression testing.

Hi, I'm not sure at which court the ball is, but in the interest at moving
things forward here's the complete patch with the testcase. OK to apply?

---8<---

From: Alexander Monakov <amonakov@ispras.ru>
Date: Fri, 13 Jan 2023 21:04:02 +0300
Subject: [PATCH] sched-deps: do not schedule pseudos across calls [PR108117]

Scheduling across calls in the pre-RA scheduler is problematic: we do
not take liveness info into account, and are thus prone to extending
lifetime of a pseudo over the loop, requiring a callee-saved hardreg
or causing a spill.

If current function called a setjmp, lifting an assignment over a call
may be incorrect if a longjmp would happen before the assignment.

Thanks to Jose Marchesi for testing on AArch64.

gcc/ChangeLog:

	PR rtl-optimization/108117
	PR rtl-optimization/108132
	* sched-deps.cc (deps_analyze_insn): Do not schedule across
	calls before reload.

gcc/testsuite/ChangeLog:

	PR rtl-optimization/108117
	PR rtl-optimization/108132
	* gcc.dg/pr108117.c: New test.
---
 gcc/sched-deps.cc               |  9 ++++++++-
 gcc/testsuite/gcc.dg/pr108117.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr108117.c

diff --git a/gcc/sched-deps.cc b/gcc/sched-deps.cc
index 948aa0c3b..5dc4fa4cd 100644
--- a/gcc/sched-deps.cc
+++ b/gcc/sched-deps.cc
@@ -3688,7 +3688,14 @@ deps_analyze_insn (class deps_desc *deps, rtx_insn *insn)
 
       CANT_MOVE (insn) = 1;
 
-      if (find_reg_note (insn, REG_SETJMP, NULL))
+      if (!reload_completed)
+	{
+	  /* Scheduling across calls may increase register pressure by extending
+	     live ranges of pseudos over the call.  Worse, in presence of setjmp
+	     it may incorrectly move up an assignment over a longjmp.  */
+	  reg_pending_barrier = MOVE_BARRIER;
+	}
+      else if (find_reg_note (insn, REG_SETJMP, NULL))
         {
           /* This is setjmp.  Assume that all registers, not just
              hard registers, may be clobbered by this call.  */
diff --git a/gcc/testsuite/gcc.dg/pr108117.c b/gcc/testsuite/gcc.dg/pr108117.c
new file mode 100644
index 000000000..ae151693e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr108117.c
@@ -0,0 +1,30 @@
+/* { dg-do run } */
+/* { dg-require-effective-target nonlocal_goto } */
+/* { dg-options "-O2 -fschedule-insns" } */
+
+#include <stdio.h>
+#include <setjmp.h>
+
+jmp_buf ex_buf;
+
+__attribute__((noipa))
+void fn_throw(int x)
+{
+   if (x)
+      longjmp(ex_buf, 1);
+}
+
+int main(void)
+{
+    int vb = 0; // NB: not volatile, not modified after setjmp
+
+    if (!setjmp(ex_buf)) {
+        fn_throw(1);
+        vb = 1; // not reached in the abstract machine
+    }
+
+    if (vb) {
+        printf("Failed, vb = %d!\n", vb);
+        return 1;
+    }
+}
-- 
2.37.2