[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #17 from CVS Commits --- The master branch has been updated by Alexander Monakov : https://gcc.gnu.org/g:733a1b777f16cd397b43a242d9c31761f66d3da8 commit r13-5154-g733a1b777f16cd397b43a242d9c31761f66d3da8 Author: Alexander Monakov Date: Fri Jan 13 21:04:02 2023 +0300 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.
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #16 from Alexander Monakov --- Draft patch for the sched1 issue: https://inbox.sourceware.org/gcc-patches/cf62c3ec-0a9e-275e-5efa-2689ff1f0...@ispras.ru/T/#m95238afa0f92daa0ba7f8651741089e7cfc03481
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 Alexander Monakov changed: What|Removed |Added Resolution|FIXED |DUPLICATE --- Comment #15 from Alexander Monakov --- Sorry, didn't mean to remove the duplicate info. I could swear I didn't touch the dropdown, not sure what happened. *** This bug has been marked as a duplicate of bug 57067 ***
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 Alexander Monakov changed: What|Removed |Added Resolution|DUPLICATE |FIXED --- Comment #14 from Alexander Monakov --- (In reply to Andrew Pinski from comment #13) > > The lifetime of the pseduo was already across the call ... Hm, I disagree: 'vb = 1' is a killing definition. Therefore the 'vb = 0' initialization is dead at the point of the call.
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #13 from Andrew Pinski --- (In reply to Alexander Monakov from comment #12) > Shouldn't there be another bug for the sched1 issue specifically? In absence > of abnormal control flow, extending lifetimes of pseudos across calls is > still likely to be a pessimization. The lifetime of the pseduo was already across the call ...
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #12 from Alexander Monakov --- Shouldn't there be another bug for the sched1 issue specifically? In absence of abnormal control flow, extending lifetimes of pseudos across calls is still likely to be a pessimization.
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #11 from Andrew Pinski --- >From the duplicated bug: "In this case the RTL scheduler pass generates broken code due to the missing control flow info."
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 Andrew Pinski changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |DUPLICATE --- Comment #10 from Andrew Pinski --- Dup of bug 57067. *** This bug has been marked as a duplicate of bug 57067 ***
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #9 from Alexander Monakov --- (In reply to Feng Xue from comment #8) > In another angle, because gcc already model control flow and SSA web for > setjmp/longjmp, explicit volatile specification is not really needed. That covers GIMPLE, but after transitioning to RTL, setjmp is not properly modeled anymore (like in old versions of GCC before Tree-SSA). Many RTL passes simply refuse touching the function if it has a setjmp call, but as your example demonstrated, scheduling still can make a surprising transform.
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #8 from Feng Xue --- (In reply to Andrew Pinski from comment #7) > (In reply to Feng Xue from comment #6) > > (In reply to Andrew Pinski from comment #2) > > > https://en.cppreference.com/w/c/program/setjmp > > > > I think that most programmers are not aware of this, neither I for sure. > > Usage of volatile here is not that intuitive as for purpose of > > multi-execution synchronization > > You should read up on > https://en.cppreference.com/w/cpp/utility/program/signal too. >From viewpoint of programmer, setjmp/longjmp is somewhat similar to C++ exception handling, which happens in one logical execution context, while signal implies two unrelated contexts. In another angle, because gcc already model control flow and SSA web for setjmp/longjmp, explicit volatile specification is not really needed. But signal mechanism is kind of asynchronous exception, for which it is impractical for compiler to do that, so volatile is must.
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #7 from Andrew Pinski --- (In reply to Feng Xue from comment #6) > (In reply to Andrew Pinski from comment #2) > > https://en.cppreference.com/w/c/program/setjmp > > I think that most programmers are not aware of this, neither I for sure. > Usage of volatile here is not that intuitive as for purpose of > multi-execution synchronization You should read up on https://en.cppreference.com/w/cpp/utility/program/signal too.
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #6 from Feng Xue --- (In reply to Andrew Pinski from comment #2) > https://en.cppreference.com/w/c/program/setjmp I think that most programmers are not aware of this, neither I for sure. Usage of volatile here is not that intuitive as for purpose of multi-execution synchronization, it just tells compiler to keep variable in memory, instead of register. With current SSA representation in GCC, probably, this could be automatically identified without explicit source level specifier. That is, any SSA name occurring in abnormal phi should go to memory during expansion to rtl.
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 Alexander Monakov changed: What|Removed |Added Status|RESOLVED|UNCONFIRMED Resolution|INVALID |--- --- Comment #5 from Alexander Monakov --- On further thought, this is really an invalid transform, because the value becomes "clobbered" only if it was changed between setjmp and longjmp: (C11 7.13.2.1 "The longjmp function") > All accessible objects have values, and all other components of the abstract > machine have state, as of the time the longjmp function was called, except > that > the values of objects of automatic storage duration that are local to the > function containing the invocation of the corresponding setjmp macro that > do not have volatile-qualified type and have been changed between the setjmp > invocation and longjmp call are indeterminate. In the testcase, the assignment 'vb = 1' did not happen in the abstract machine. Moving back to UNCONFIRMED, both because the transform is invalid, and because lifting assignments to pseudos across calls in sched1 seems useless if not harmful to performance and code size. (that said, the -Wclobbered diagnostic still points to a potential issue, so it shouldn't be ignored)
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 Alexander Monakov changed: What|Removed |Added CC||amonakov at gcc dot gnu.org --- Comment #4 from Alexander Monakov --- -Wclobbered properly warns here (and it's part of -Wextra). With explicit -fschedule-insns, reproducible on x86 as well. The reason for the issue is quite surprising though, I did not expect pre-RA scheduling to lift assignments to pseudos across calls, because it just increases register pressure at the point of the call for little or no gain.
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #3 from Andrew Pinski --- There used to be a warning for this. Even saw someone post an updated version of it that happens on gimple (but that never went in).
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 --- Comment #2 from Andrew Pinski --- https://en.cppreference.com/w/c/program/setjmp
[Bug rtl-optimization/108117] Wrong instruction scheduling on value coming from abnormal SSA
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108117 Andrew Pinski changed: What|Removed |Added Resolution|--- |INVALID Status|UNCONFIRMED |RESOLVED --- Comment #1 from Andrew Pinski --- Vb needs to be marked as volatile.
