From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.84]) by sourceware.org (Postfix) with ESMTPS id 39E693858C39 for ; Fri, 13 Jan 2023 18:20:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 39E693858C39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=ispras.ru Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=ispras.ru Received: from [10.10.3.121] (unknown [10.10.3.121]) by mail.ispras.ru (Postfix) with ESMTPS id 7CAAA40737DF; Fri, 13 Jan 2023 18:20:12 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru 7CAAA40737DF Date: Fri, 13 Jan 2023 21:20:12 +0300 (MSK) From: Alexander Monakov To: "Jose E. Marchesi" cc: Alexander Monakov via Gcc-patches , qing.zhao@oracle.com, richard.sandiford@arm.com Subject: [PATCH] sched-deps: do not schedule pseudos across calls [PR108117] In-Reply-To: <87h6xm1jvj.fsf@oracle.com> Message-ID: <0cc6e188-c64c-e8ea-83e4-1d06f5bf4f55@ispras.ru> References: <20221222173208.13317-1-jose.marchesi@oracle.com> <53b93d7e-a157-9116-d07a-4d51cd43d205@ispras.ru> <87h6xm1jvj.fsf@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-9.1 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 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 +#include + +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