Hi Jin, In case you missed it, we proposed something similar: https://gcc.gnu.org/pipermail/gcc-patches/2026-August/726438.html Perhaps there is something useful in there you can adapt, I didn't compare the implementations in detail though.
>> What you're doing inhibits that case and fusing of >> stores into pairs is one of the most important fusion cases, hence my >> focus on getting those right. In fact, I would claim that getting +1 for this. Our design has different restrictions. 8 byte loads don't fuse, but 2 and 4 byte do. Sorry for the late reply, only noticed this patch now. Kind regards, Michiel "Jin Ma" <[email protected]> writes: >> On 8/31/26 9:31 AM, Jin Ma wrote: >> >> 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. >> Note that the Veyron V2 can fuse an arbitrary pair of same sized >> adjacent stores. What you're doing inhibits that case and fusing of >> stores into pairs is one of the most important fusion cases, hence my >> focus on getting those right. In fact, I would claim that getting >> stores and the address->load cases is all that *really* matters in the >> end for DEC based pair fusion. >> >> Jeff > > Hi Jeff, > > Thank you for the clarification. You are right. I am very sorry that > I had considered only the newly added RISCV_FUSE_LDST_PAIR_INC/DEC and > RISCV_FUSE_FLDFST_PAIR_INC/DEC operations and overlooked the existing > RISCV_FUSE_ALIGNED_STD. I will include it in the priority hook and in > the set of operations that enable scheduling fusion by default, while > retaining the 4/8-byte restriction only for operations that require it. > > I noticed that Patchwork has marked this patch as Accepted and archived > it. Would you prefer that I post a revised version with this change for > review, or may I make this adjustment before committing it to trunk > without reposting the patch? > > Thanks, > Jin
