On 8/16/25 4:16 AM, Artemiy Volkov wrote:
On Sun, Aug 10, 2025 at 09:28:50AM -0600, Jeff Law wrote:
Conceptually OK. I slightly worry about the use of PREV_INSN as we could
conceptually end up with notes, line markers, etc between the two fusible
insns.
Right now the scheduler is the only pass that ever examines SCHED_GROUP_P
and, IIRC, we remove all the notes from the IL, schedule, then put all the
notes back in. Point being within the scheduler PREV_INSN is always going
to get us the previous real insn. Outside the scheduler context PREV_INSN
might not do exactly what we want and may even introduce compare-debug
failures.
ISTM it's probably safer to use prev_nonnote_nondebug_insn. The biggest
worry with using that API is walking past a BB marker. But in the fusion
case, I think we're going to be safe from those problems.
Given the simplicity of this patch, I'd be inclined to ACK it independent of
the main body of work, so if you could bootstrap and regression test it in
isolation after adjusting the PREV_INSN uses it'd be appreciated.
Hi Jeff, thanks for taking a look.
Please find a new version of this patch attached. I've bootstrapped and
regtested it in isolation and with patch #3 on x86_64, i386, aarch64,
and riscv64, and nothing came up. I've also done the same on top of
patch #1 (which provides quite a lot more coverage), and everything
passed as well.
THanks. I've pushed this to the trunk.
jeff
