> On 8/17/26 12:19 AM, Jin Ma wrote:
> > The sched_fusion pass can bring independent memory accesses together
> > before sched2. Add a priority hook that groups scalar load/store-pair
> > candidates by register file, access kind, mode and base register, then
> > orders each group by offset. Prefer decreasing offsets when both
> > directions are available, matching frame save/restore memory order.
> >
> > Enable scheduling fusion by default for load/store-pair tunes while
> > preserving an explicit -f[no-]schedule-fusion option. Keep the positive
> > test as XFAIL until the tuning patch enables the pairs.
> >
> > Static sched2 macro-fusion counts for SPEC CPU 2017 Integer built with
> > -O3 -flto, measured independently of ready-list reordering against a
> > baseline with xt-c9501fdvt fusion pairs enabled:
> >
> > Benchmark Baseline With patch Change
> > 500.perlbench_r 15302 17077 +11.6%
> > 502.gcc_r 72806 78135 +7.3%
> > 505.mcf_r 63 85 +34.9%
> > 520.omnetpp_r 20479 22084 +7.8%
> > 523.xalancbmk_r 42442 44651 +5.2%
> > 525.x264_r 6232 7033 +12.9%
> > 531.deepsjeng_r 299 328 +9.7%
> > 541.leela_r 794 851 +7.2%
> > 548.exchange2_r 353 426 +20.7%
> > 557.xz_r 1217 1321 +8.5%
> > SUM 159987 171991 +7.5%
> >
> > gcc/ChangeLog:
> >
> > * config/riscv/riscv-fusion.cc (riscv_sched_fusion_priority): New
> > function.
> > * config/riscv/riscv-protos.h (riscv_sched_fusion_priority):
> > Declare.
> > * config/riscv/riscv.cc (riscv_override_options_internal): Set the
> > scheduling-fusion default for the current tune.
> > (TARGET_SCHED_FUSION_PRIORITY): Define.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/riscv/sched-fusion-priority.c: New test.
> >
> > Signed-off-by: Jin Ma <[email protected]>
>
> > +
> > + isload_p = mem.type != SCHED_FUSION_ST;
> > + fp_p = mem.fp_p;
> > + access_size = GET_MODE_SIZE (mem.mode).to_constant ();
> > + if (access_size != 4 && access_size != 8)
> > + return;
> In an ideal world we'd have a way to describe the access size stuff
> better since that restriction is a function of the uarch and some don't
> have the access size restrictions. Could we perhaps query which of the
> fusions are enabled to determine which access sizes should be allowed?
>
> More generally it seems like we should test that the two instructions
> are fusible before we do any priority adjustments. I don't have
> get_fusible_ops in front of me right now. If it's doing that check,
> then we're likely good here (and I think that would eliminate the need
> to test access sizes in this routine since the check that the insns
> could be fused would be false and we'd exit without any priority
> adjustments).
>
> Jeff
Hi Jeff,
Thank you for the review.
riscv_get_fusible_ops only returns the fusion mask enabled by the
current tune. It does not check whether a specific pair of instructions
can be fused.
Under the current definitions, the access width is part of the semantics
of these pair operations. RISCV_FUSE_LDST_PAIR_INC/DEC describes 4- or
8-byte integer accesses, while RISCV_FUSE_FLDFST_PAIR_INC/DEC describes
4- or 8-byte floating-point accesses. Thus, the pair operation and the
4/8-byte restriction are currently coupled rather than being independent
microarchitectural properties. The priority hook already uses the
enabled operation to select an integer or floating-point pair and its
direction. The width check ensures that it adjusts only candidates
supported by that operation. Removing the check would also give byte
and halfword accesses a pair priority, although the existing pair
matcher would reject them.
I think this is sufficient for the currently defined pairs. When both
ascending and descending integer pairs are enabled, the current code
prefers decreasing addresses. For example, given these independent
instructions:
ld a1,0(a0)
add a3,a4,a5
ld a2,8(a0)
the priority pass can bring the two 8-byte loads together as follows:
ld a2,8(a0)
ld a1,0(a0)
add a3,a4,a5
The final matcher then recognizes the first two instructions as
RISCV_FUSE_LDST_PAIR_DEC, based on their common base, 8-byte access
width, and offset difference of 8. If only the ascending pair is
enabled, it instead prefers the 0(a0), 8(a0) order. Conversely, lb and
lh do not receive this pair priority because the current operations do
not support 1- or 2-byte accesses.
TARGET_SCHED_FUSION_PRIORITY receives only one instruction per call, so
there is no PREV/CURR pair to pass to the two-instruction matcher. The
purpose of this pass is to bring independent candidates that are not yet
adjacent together. It can therefore only classify an individual
candidate; the final two-instruction matcher still checks the common
base, adjacent offsets, and register relationships exactly.
Since all the relevant operations currently have the same 4/8-byte
restriction, I would prefer to keep the current implementation simple.
For example, if a future core also supported byte or halfword load/store
pairs while C950 remained limited to 4/8-byte accesses, we could add a
pair operation describing those additional widths and accept 1- or
2-byte accesses in its matcher. The priority hook would assign pair
priorities to those accesses only when that operation was enabled.
I am not entirely sure that I have understood your concern correctly.
Do you have any further comments or suggestions?
BR,
Jin
