When compiling the attached code, with an arm-none-eabi cross compiler from trunk, arm-none-eabi-gcc -march=armv6-m -S test.c -o test.s -Os incorrect assembly is generated, which leads to the second assert always being triggered. This happens since revision r266734 which introduced a new pass running jump-threading just after reload. For the attached testcase this triggers a latent bug in the `thread_jump` function. The combine pass can modify a jump_insn so that its pattern is of the form (parallel [ (set (pc) ...) (clobber (scratch))]) which after reload can end up in the form (parallel [ (set (pc) (if_then_else (<something including (reg N)) ...)) (clobber (reg N))]) where 'N' is the same register. The `thread_jump` function does not account for this possibility occurring at the last insn of a basic block. When checking to see what modifications the basic block makes to registers (using `mark_effect`), it records that the block doesn't overall set (reg N) since it is clobbered in the last insn. It then incorrectly deduces from this fact that the jump condition using (reg N) is not affected by the execution of the basic block and can hence be skipped (using `mentions_nonequal_regs`). I am currently working on a patch, but reporting upstream so it's on peoples radar. n.b. I am currently looking into how `onlyjump_p` is used elsewhere to see whether it should account for the possibility of a CLOBBER in a PARALLEL pattern or whether the fix should be constrained to how the `thread_jump` function calculates what registers are used in the final jump condition.
Created attachment 45458 [details] Problematic testcase
I think the right fix is something like: --- gcc/cfgcleanup.c.jj 2019-01-01 12:37:19.147942300 +0100 +++ gcc/cfgcleanup.c 2019-01-21 16:35:00.737320489 +0100 @@ -338,6 +338,13 @@ thread_jump (edge e, basic_block b) insn != NEXT_INSN (BB_END (b)) && !failed; insn = NEXT_INSN (insn)) { + /* cond2 must not mention any register that is not equal to the + former block. Check this before processing that instruction, + as BB_END (b) could contain also clobbers. */ + if (insn == BB_END (b) + && mentions_nonequal_regs (cond2, nonequal)) + goto failed_exit; + if (INSN_P (insn)) { rtx pat = PATTERN (insn); @@ -362,11 +369,6 @@ thread_jump (edge e, basic_block b) goto failed_exit; } - /* cond2 must not mention any register that is not equal to the - former block. */ - if (mentions_nonequal_regs (cond2, nonequal)) - goto failed_exit; - EXECUTE_IF_SET_IN_REG_SET (nonequal, 0, i, rsi) goto failed_exit; i.e. verify the condition before processing the second conditional jump, because it is evaluated with the state of registers before the instruction, rather than after it.
I agree Jakub -- I've been testing a patch that does the same thing and everything seems to be working (though my patch was not as neat).
Created attachment 45482 [details] gcc9-pr88904.patch Full untested patch.
Author: jakub Date: Tue Jan 22 09:12:31 2019 New Revision: 268140 URL: https://gcc.gnu.org/viewcvs?rev=268140&root=gcc&view=rev Log: PR rtl-optimization/88904 * cfgcleanup.c (thread_jump): Verify cond2 doesn't mention any nonequal registers before processing BB_END (b). * gcc.c-torture/execute/pr88904.c: New test. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr88904.c Modified: trunk/gcc/ChangeLog trunk/gcc/cfgcleanup.c trunk/gcc/testsuite/ChangeLog
Fixed.