On Fri, Sep 27, 2024 at 2:11 PM Andre Vieira (lists) <andre.simoesdiasvie...@arm.com> wrote: > > > > On 26/09/2024 18:56, Ramana Radhakrishnan wrote: > > > > >> +/* Helper function to determine whether SEQ represents a sequence of > >> + instructions representing the Armv8.1-M Mainline conditional arithmetic > >> + instructions: csinc, csneg and csinv. The cinc instruction is generated > >> + using a different mechanism. */ > >> + > >> +static bool > >> +arm_is_v81m_cond_insn (rtx_insn *seq) > >> +{ > >> + rtx_insn *curr_insn = seq; > >> + rtx set; > >> + /* The pattern may start with a simple set with register operands. Skip > >> + through any of those. */ > >> + while (curr_insn) > >> + { > >> + set = single_set (curr_insn); > >> + if (!set > >> + || !REG_P (SET_DEST (set))) > >> + return false; > >> + > >> + if (!REG_P (SET_SRC (set))) > >> + break; > >> + curr_insn = NEXT_INSN (curr_insn); > > > > Too late at night for me - but don’t you want to skip DEBUG_INSNS in some > > way here ? > > > It's a good point, but this sequence is created by noce as a potential > replacement for the incoming one and no debug insns are inserted here.
True and fair. > > Compiling gcc/gcc/testsuite/gcc.target/arm/csinv-1.c with -g3 for an > Armv8.1-M Mainline target still generates the csinv. > > Either way, I could add code to skip if we don't have a NONDEBUG_INSN_P, > but that means we should also do so after every NEXT_INSN after the > while loop and at the end. It does feel 'more robust', but I also fear > it might be a bit overkill here? Feels overkill as you say. Please watch out for any regressions Ok to commit and later backport if no regressions as this is sufficiently isolated to only Armv8.1M configurations. regards Ramana > > Kind regards, > Andre > >