On 09/06/2026 06:01, [email protected] wrote: > 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). >
I've pushed this, thanks for cleaning up the patch. I did make a couple of minor changes, though: - ChangeLog fragments need to be indented using tabs, not spaces. - I've adjusted the test to use arm_arch_v8m_base as the effective target. The test is for CBZ and that was only added in armv8m.base. The test now adds the relevant flags to ensure this. Although I didn't see failures when the test runs on Arm or other targets, that's more by luck than by design, so we shouldn't be relying on that. >> >> 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"? > I've added you to the collaborators team. Thanks once again, R.> 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" } } */ >>> >> >
