https://gcc.gnu.org/g:3f4f26849bc42374d9b07df045cd172bd37dfbfa
commit r17-676-g3f4f26849bc42374d9b07df045cd172bd37dfbfa Author: Richard Sandiford <[email protected]> Date: Fri May 22 16:27:31 2026 +0100 cfgrtl: Forbid forwarder blocks from having clobbers [PR125375] In this testcase, jump2 was presented with: L1: set the return register do epilogue stuff goto L4 L2: do the same epilogue stuff L3: clobber the return register goto L4 The question then is: is the L3 block a forwarder block? It is a forwarder block in the sense that a jump to L3 can be replaced with a jump to L4. But it isn't a forwarder block in the sense of a jump to L3 being equivalent to a jump to L4. In particular, a jump to L4 cannot be merged with a jump or fallthrough to L3 unless we can prove that the clobber is valid for both paths. In the testcase, L3 was marked as a forwarder block and so cross-jumping created: L1: set the return register L2: do epilogue stuff L3: clobber the return register goto L4 The set of the return register was then inevitably deleted as dead. The clobber in this case is of the return register. But the same principle/problem would apply to any clobber. We can't introduce new clobbers on a path without proving that the clobbered thing is dead. This question arises due to an old quirk of active_insn_p that predates CVS history: bool active_insn_p (const rtx_insn *insn) { return (CALL_P (insn) || JUMP_P (insn) || JUMP_TABLE_DATA_P (insn) /* FIXME */ || (NONJUMP_INSN_P (insn) && (! reload_completed || (GET_CODE (PATTERN (insn)) != USE && GET_CODE (PATTERN (insn)) != CLOBBER)))); } Thus a clobber is "active" before RA but not after it. This means that, according to flow_active_insn_p, a block with a clobber is not a forwarder block before RA, but can be afterwards. The "most optimal" solution would probably be to split the concept of forwarder block into two, one that allows clobbers and one that doesn't. However, that would be difficult to retrofit at this stage and isn't likely to be suitable for backporting. This patch therefore takes the more conservative approach of making flow_active_insn_p treat clobbers in the same way after RA as it does before RA. Some of this infrastructure is probably ripe for updating. For example, flow might have required explicit uses of the return register, but DF should cope well enough without. We should probably also check whether the active_insn_p behaviour still makes sense. gcc/ PR rtl-optimization/125375 * cfgrtl.cc (flow_active_insn_p): Return true for clobbers. gcc/testsuite/ * gcc.dg/pr125375.c: New test. Diff: --- gcc/cfgrtl.cc | 37 +++++++++++++++++++++++++------------ gcc/testsuite/gcc.dg/pr125375.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/gcc/cfgrtl.cc b/gcc/cfgrtl.cc index 0ec72f08fa87..474b84a08934 100644 --- a/gcc/cfgrtl.cc +++ b/gcc/cfgrtl.cc @@ -558,8 +558,8 @@ update_bb_for_insn (basic_block bb) } -/* Like active_insn_p, except keep the return value use or clobber around - even after reload. */ +/* Return true if adding or removing instances of INSN might affect the + semantics of the RTL. */ static bool flow_active_insn_p (const rtx_insn *insn) @@ -567,16 +567,29 @@ flow_active_insn_p (const rtx_insn *insn) if (active_insn_p (insn)) return true; - /* A clobber of the function return value exists for buggy - programs that fail to return a value. Its effect is to - keep the return value from being live across the entire - function. If we allow it to be skipped, we introduce the - possibility for register lifetime confusion. - Similarly, keep a USE of the function return value, otherwise - the USE is dropped and we could fail to thread jump if USE - appears on some paths and not on others, see PR90257. */ - if ((GET_CODE (PATTERN (insn)) == CLOBBER - || GET_CODE (PATTERN (insn)) == USE) + /* We cannot add new clobbers to a path without first proving that + the clobbered thing is dead. + + For example: + + (code_label L1) + (clobber X) + (set (pc) (label_ref L2)) + + is a forwarder block in the sense that a jump to L1 can be replaced + with a jump to L2, although perhaps with some loss of dataflow precision. + However, any attempt to merge a jump to L1 with a jump to L2 would be an + asymmetric operation, in that the merged code must jump to L2 rather than + to L1. Our current definition of "forwarder block" does not allow for + this distinction and so we need to take a conservatively correct + approach. */ + if (GET_CODE (PATTERN (insn)) == CLOBBER) + return true; + + /* Keep a USE of the function return value, otherwise the USE is dropped and + we could fail to thread a jump if USE appears on some paths and not on + others, see PR90257. */ + if (GET_CODE (PATTERN (insn)) == USE && REG_P (XEXP (PATTERN (insn), 0)) && REG_FUNCTION_VALUE_P (XEXP (PATTERN (insn), 0))) return true; diff --git a/gcc/testsuite/gcc.dg/pr125375.c b/gcc/testsuite/gcc.dg/pr125375.c new file mode 100644 index 000000000000..512d23778585 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr125375.c @@ -0,0 +1,29 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-inline -fno-compare-elim" } */ + +int a, c = 1; +short b = 1, e; +volatile unsigned short d; +static int f() { + unsigned g = 2; + if (c >= a) + if ((c || b) && b) { + unsigned h = c && d; + int i = e = d; + if (d) + i = d = 0; + g = c ^ g * i; + c = ~c; + b = b * (g | 9) & ((1 && a) - i); + h && i && d; + a = c ^ e ^ (g && a) * h; + d = e; + if (a) + return g; + } +} +int main() { + if (f() != 1) + __builtin_abort(); + return 0; +}
