On Mon, 2026-06-08 at 14:44 +0100, Richard Earnshaw wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > On 29/05/2026 08:03, [email protected] wrote: > > Hello, > > > > thumb1_cbz uses operands[2], both when checking if it can reuse the > > previously set condition code, and when recording the operands that > > set the current condition code. operands[2] for this pattern is the > > target label of the jump though, and not the intended second operand > > of the comparison operator. > > > > As the label is unlikely to end up as operands[2], the condition code > > reuse logic doesn't kick in and causes the redundant CMP instruction > > described in PR124077. In addition, thumb1_final_prescan_insn also > > doesn't treat thumb1_cbz as a cbranch and ends up resetting condition > > code tracking state. > > > > This patch fixes this by replacing operands[2] with the actual second > > operand of the pattern i.e const0_rtx. It also treats thumb1_cbz the > > same as cbranchsi4_insn in thumb1_final_prescan_insn and lets it track > > the condition code state itself. > > > > Ok for master? > > I was going to have a look at this today, but the patch appears to be > corrupt. Investigation suggests that your mailer has converted all the > hard tabs in the original code into spaces, so the patch will not apply.
Apologies for wasting your time. I've attached the patch this time, and verified it applies cleanly on gcc master). > > If you're having problems with your mail setup for sending patches, I'd > be happy to look at arm-related patches that are posted on our > experimental forge instance: forge.sourceware.org/gcc/gcc-TEST. I have a few more patches in the queue, so I thought I'd try using forge for them. Following https://gcc.gnu.org/wiki/UsingForge, I created an account - can you please add https://forge.sourceware.org/saaadhu to the "right team"? Regards Senthil > > R. > > > > > Regards > > Senthil > > > > gcc/ChangeLog: > > > > PR target/124077 > > * config/arm/arm.cc (thumb1_final_prescan_insn): Also skip > > condition code update for thumb1_cbz instructions. > > * config/arm/thumb1.md (thumb1_cbz): Use const0_rtx as the > > recorded cc_op1 instead of operands[2]. > > > > gcc/testsuite/ChangeLog: > > > > PR target/124077 > > * gcc.target/arm/pr124077.c: New test. > > > > > > diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc > > index 41808e42ab1..ec46cac0e61 100644 > > --- a/gcc/config/arm/arm.cc > > +++ b/gcc/config/arm/arm.cc > > @@ -26612,7 +26612,8 @@ thumb1_final_prescan_insn (rtx_insn *insn) > > asm_fprintf (asm_out_file, "%@ 0x%04x\n", > > INSN_ADDRESSES (INSN_UID (insn))); > > /* Don't overwrite the previous setter when we get to a cbranch. */ > > - if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn) > > + if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn > > + && INSN_CODE (insn) != CODE_FOR_thumb1_cbz) > > { > > enum attr_conds conds; > > > > diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md > > index db0fab2e1f6..4fa64e42340 100644 > > --- a/gcc/config/arm/thumb1.md > > +++ b/gcc/config/arm/thumb1.md > > @@ -1120,7 +1120,7 @@ > > if (t != NULL_RTX) > > { > > if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) > > - || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) > > + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, const0_rtx)) > > t = NULL_RTX; > > if (cfun->machine->thumb1_cc_mode == CC_NZmode) > > { > > @@ -1135,7 +1135,7 @@ > > output_asm_insn ("cmp\t%1, #0", operands); > > cfun->machine->thumb1_cc_insn = insn; > > cfun->machine->thumb1_cc_op0 = operands[1]; > > - cfun->machine->thumb1_cc_op1 = operands[2]; > > + cfun->machine->thumb1_cc_op1 = const0_rtx; > > cfun->machine->thumb1_cc_mode = CCmode; > > } > > else > > diff --git a/gcc/testsuite/gcc.target/arm/pr124077.c > > b/gcc/testsuite/gcc.target/arm/pr124077.c > > new file mode 100644 > > index 00000000000..8841629a928 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/arm/pr124077.c > > @@ -0,0 +1,24 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-Os" } */ > > +/* { dg-require-effective-target arm_thumb1_ok } */ > > + > > +extern int y; > > +extern void bar(void); > > +extern volatile int *p; > > + > > +#define FILL p[0] = 1; p[1] = 2; p[2] = 3; p[3] = 4; p[4] = 5; p[5] = 6; > > p[6] = 7; > > + > > +void foo(int x, int z) > > +{ > > + y = x & z; > > + if (y) > > + { > > + FILL FILL FILL FILL FILL; > > + } > > + else > > + { > > + bar(); > > + } > > +} > > + > > +/* { dg-final { scan-assembler-not "cmp\tr\[0-9\], #0" } } */ > > >
commit 20ecaef386d0a530348db3c620c9c1d8b6a8e17d Author: Ciprian Arbone <[email protected]> Date: Wed May 27 09:49:29 2026 +0530 ARM: Fix broken Thumb1 CBZ cc tracking [PR124077] thumb1_cbz uses operands[2], both when checking if it can reuse the previously set condition code, and when recording the operands that set the current condition code. operands[2] for this pattern is the target label of the jump though, and not the intended second operand of the comparison operator. As the label is unlikely to end up as operands[2], the condition code reuse logic doesn't kick in and causes the redundant CMP instruction described in PR124077. In addition, thumb1_final_prescan_insn also doesn't treat thumb1_cbz as a cbranch and ends up resetting condition code tracking state. Fix by replacing operands[2] with the actual second operand of the pattern i.e const0_rtx. Also treat thumb1_cbz the same as cbranchsi4_insn in thumb1_final_prescan_insn and let it track condition code state itself. gcc/ChangeLog: PR target/124077 * config/arm/arm.cc (thumb1_final_prescan_insn): Also skip condition code update for thumb1_cbz instructions. * config/arm/thumb1.md (thumb1_cbz): Use const0_rtx as the recorded cc_op1 instead of operands[2]. gcc/testsuite/ChangeLog: PR target/124077 * gcc.target/arm/pr124077.c: New test. Co-authored-by: Senthil Kumar Selvaraj <[email protected]> diff --git gcc/config/arm/arm.cc gcc/config/arm/arm.cc index 971347ab37e..d24b06a6445 100644 --- gcc/config/arm/arm.cc +++ gcc/config/arm/arm.cc @@ -26834,7 +26834,8 @@ thumb1_final_prescan_insn (rtx_insn *insn) asm_fprintf (asm_out_file, "%@ 0x%04x\n", INSN_ADDRESSES (INSN_UID (insn))); /* Don't overwrite the previous setter when we get to a cbranch. */ - if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn) + if (INSN_CODE (insn) != CODE_FOR_cbranchsi4_insn + && INSN_CODE (insn) != CODE_FOR_thumb1_cbz) { enum attr_conds conds; diff --git gcc/config/arm/thumb1.md gcc/config/arm/thumb1.md index 611d8b88502..0c704c5d87f 100644 --- gcc/config/arm/thumb1.md +++ gcc/config/arm/thumb1.md @@ -1120,7 +1120,7 @@ if (t != NULL_RTX) { if (!rtx_equal_p (cfun->machine->thumb1_cc_op0, operands[1]) - || !rtx_equal_p (cfun->machine->thumb1_cc_op1, operands[2])) + || !rtx_equal_p (cfun->machine->thumb1_cc_op1, const0_rtx)) t = NULL_RTX; if (cfun->machine->thumb1_cc_mode == CC_NZmode) { @@ -1135,7 +1135,7 @@ output_asm_insn ("cmp\t%1, #0", operands); cfun->machine->thumb1_cc_insn = insn; cfun->machine->thumb1_cc_op0 = operands[1]; - cfun->machine->thumb1_cc_op1 = operands[2]; + cfun->machine->thumb1_cc_op1 = const0_rtx; cfun->machine->thumb1_cc_mode = CCmode; } else diff --git gcc/testsuite/gcc.target/arm/pr124077.c gcc/testsuite/gcc.target/arm/pr124077.c new file mode 100644 index 00000000000..8841629a928 --- /dev/null +++ gcc/testsuite/gcc.target/arm/pr124077.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-Os" } */ +/* { dg-require-effective-target arm_thumb1_ok } */ + +extern int y; +extern void bar(void); +extern volatile int *p; + +#define FILL p[0] = 1; p[1] = 2; p[2] = 3; p[3] = 4; p[4] = 5; p[5] = 6; p[6] = 7; + +void foo(int x, int z) +{ + y = x & z; + if (y) + { + FILL FILL FILL FILL FILL; + } + else + { + bar(); + } +} + +/* { dg-final { scan-assembler-not "cmp\tr\[0-9\], #0" } } */
