Hi Jeff, Jin, > I have not found an ideal way to balance accurate modeling with generic > reuse. Could you suggest what abstraction GCC would prefer in this > case, or whether there is a better way to represent these constraint > differences between microarchitectures?
We have encountered the same issue. Many of the pre-existing fusion matchers almost fit our needs but can't be reused because of differing constraints. Currently we are trying to reuse as much as possible by A) outlining parts of the matcher that are the same; e.g. reuse the RTL maching but separate destination register checking B) splitting up matchers; e.g; split up BFEXT into one for srli and srai However, splitting up some fusion matchers to ensure maximum reusability can make things needlessly complicated and we just accept the overlap. I propose to focus for now on providing generic building blocks (util functions) for building matchers and not focus too much on making the RISCV_FUSE_* enum itself reusable. Somewhat related: I'm planning on making RA aware of these register constraints and then these would need to be separated out anyway. Haven't checked yet whether that's actually worth the effort though. Thanks, Michiel ________________________________________ From: Jin Ma <[email protected]> Sent: Wednesday, July 22, 2026 10:17 AM To: [email protected] <[email protected]>; [email protected] <[email protected]> Cc: [email protected] <[email protected]>; Derhaeg, Michiel <[email protected]>; Jin Ma <[email protected]> Subject: Re: [PATCH v3 4/5] RISC-V: Add macro-fusion pair recognition Hi Jeff, Thank you for the review. I understand the concern. These new pairs are in the same broad categories as existing fusions, but their register dependencies and accepted RTL forms are not the same. > RISCV_FUSE_ADD_LD - add type + integer load > RISCV_FUSE_ADD_ST - add type + integer store > Aren't these already handled by existing cases? RISCV_FUSE_LDINDEXED and RISCV_FUSE_EXPANDED_LD both require the add destination and the load destination to be the same register. The new RISCV_FUSE_ADD_LD instead requires these registers to be different, so the key register relationships are opposite. There is no corresponding existing add-and-store case, and the new recognizers also need to handle addw and add.uw. A minimal RTL comparison is: Existing load fusion: (set (reg rd1) (plus (reg rs1) (reg rs2))) (set (reg rd1) (mem (reg rd1))) New ADD_LD: (set (reg rd1) (plus (reg rs1) (reg rs2))) (set (reg rd2) (mem (reg rd1))) where rd1 != rd2. New ADD_ST: (set (reg rd1) (plus (reg rs1) (reg rs2))) (set (mem (reg rd1)) (reg rs3)) where rd1 != rs3. I do not understand why the existing load fusion requires the load base and destination to be the same register. From a general dependency perspective, this constraint seems unusual to me. However, it may reflect the requirements of another vendor's microarchitecture. Without the relevant hardware specification or confirmation from that vendor, I do not think it is appropriate to change or relax the existing semantics. Enabling the existing load fusions for C950 would therefore accept a same-destination form that C950 does not support, while missing the distinct-destination form that it does support. > RISCV_FUSE_SLLI_SRLI - slli/slliw + srli/srliw > Similarly. These are bitfield extractions. For non-word slli + srli, RISCV_FUSE_SLLI_SRLI does overlap with RISCV_FUSE_BFEXT. However, RISCV_FUSE_BFEXT also recognizes slli + srai, and it does not fully recognize the extension-wrapped RTL used for slliw + srliw. C950 supports only the logical-right-shift form, and requires the two instructions to be either both word operations or both non-word operations. For example, RISCV_FUSE_BFEXT also accepts: (set (reg rd1) (ashift (reg rs1) (const_int shamt1))) (set (reg rd1) (ashiftrt (reg rd1) (const_int shamt2))) The new pair also needs to recognize the following word RTL: (set (reg:DI rd1) (sign_extend:DI (ashift:SI (reg:SI rs1) (const_int shamt1)))) (set (reg:DI rd1) (zero_extend:DI (lshiftrt:SI (reg:SI rd1) (const_int shamt2)))) Enabling RISCV_FUSE_BFEXT directly would therefore report slli + srai, which is not in the C950 specification, while missing the word form. This is the most substantial overlap among these cases. The common part is the slli + srli RTL matching; the differences are the word form and whether an arithmetic right shift is allowed. > RISCV_FUSE_PREINDEX_LD - addi type + load > RISCV_FUSE_PREINDEX_ST - addi type + store > Again, feels like this likely mirror existing functionality. RISCV_FUSE_LDPREINCREMENT supports only integer loads and requires the updated base register and the load destination to be the same register. The new pre-index load requires them to be different. The new cases also cover stores, floating-point loads and stores, self-moves, and lo_sum forms. Thus, the principal register relationship for the existing and new load forms is again opposite. For example: Existing LDPREINCREMENT: (set (reg rd1) (plus (reg rd1) (const_int imm12))) (set (reg rd1) (mem (reg rd1))) New PREINDEX_LD: (set (reg rd1) (plus (reg rd1) (const_int imm12))) (set (reg rd2) (mem (reg rd1))) where rd1 != rd2. New PREINDEX_ST: (set (reg rd1) (plus (reg rd1) (const_int imm12))) (set (mem (reg rd1)) (reg rs1)) where rd1 != rs1. > RISCV_FUSE_LDST_PAIR_INC - ascending integer load/store pair > RISCV_FUSE_LDST_PAIR_DEC - descending integer load/store pair > And these as well. There is real overlap here: pair-aligned SI/DI store pairs may be accepted by both an existing recognizer and a new recognizer. However, their full accepted sets differ. RISCV_FUSE_CACHE_ALIGNED_STD handles only DI store pairs. It requires the base to be known to have at least 16-byte alignment, requires the smaller offset to be 16-byte aligned, and normalizes the offset order. RISCV_FUSE_ALIGNED_STD also handles only store pairs, but accepts same-width stores in scalar integer modes. It requires the smaller offset to be a multiple of twice the access size and is also insensitive to instruction order. RISCV_FUSE_LDST_PAIR_INC and RISCV_FUSE_LDST_PAIR_DEC handle both loads and stores, accept only 4-byte and 8-byte accesses, do not require pair alignment, and preserve ascending and descending instruction order as separate capabilities. For loads, they also reject zero-extending forms and enforce destination and base-register constraints. For example, these two SI loads form a valid ascending pair, although the smaller offset is 4 and is not aligned to the 8-byte pair boundary: (set (reg:SI rd1) (mem:SI (plus (reg:DI rs1) (const_int 4)))) (set (reg:SI rd2) (mem:SI (plus (reg:DI rs1) (const_int 8)))) Here rd1 and rd2 are nonzero and distinct, and rd1 != rs1. Likewise, only the new ascending checker accepts this SI store pair: (set (mem:SI (plus (reg:DI rs1) (const_int 4))) (reg:SI rs2)) (set (mem:SI (plus (reg:DI rs1) (const_int 8))) (reg:SI rs3)) The descending pair contains the same addresses in offset-8, offset-4 order. Enabling RISCV_FUSE_ALIGNED_STD directly would not only miss load pairs and SI/DI pairs without pair alignment, but would also overmatch byte and halfword store pairs that C950 does not support. Conversely, broadening the existing checker would give a model that is too broad for CPUs that support only its original width or alignment constraints. > The design goal here is the enum describes a set of fairly generic > fusion opportunities, then each core sets those it supports. Having > multiple implementations of what is effectively the same fusion case > seems undesirable, hence my concern about defining fusions that I > think already exist. The current approach does not allocate an independent bit for every difference. It reuses an existing capability when the differences are acceptable. For example, the C950 definitions of LUI/AUIPC + ADDI type are not identical to the existing RISCV_FUSE_LUI_ADDI and RISCV_FUSE_AUIPC_ADDI definitions. After evaluating the RTL accepted by the existing checkers and the resulting impact, those differences were considered acceptable, so the existing bits are reused here. On the other hand, further microarchitectures may have additional subtle differences that cannot be ignored and need independent representation. The preceding patch widened the fusion bitmask to unsigned HOST_WIDE_INT so that the current width would not limit how such capabilities can be represented. The wider bitmask does not itself determine how finely the capabilities should be divided. The purpose of a new bit is not to make the accepted sets of different checkers disjoint. It is to represent a vendor-neutral set of hardware constraints that an existing bit cannot model accurately. A new bit is appropriate only when that difference forms an independently definable capability that may also be reusable by other CPUs. Some RTL overlap between checkers is acceptable. The one-bit-per-checker relationship is intended only to keep enablement, recognition, and the dump name in a clear one-to-one relationship, not to partition the RTL space into disjoint sets. Shared RTL parsing, register-dependency checks, and address checks are a separate issue from whether the capabilities themselves are independent. What I am unsure about is how to choose the right capability boundary for these subtle but real differences between microarchitectures. If a generic fusion bit is too broad, only CPUs supporting all of its RTL forms can safely enable it. If every subtle difference receives an independent bit, the definitions become fragmented and less reusable. I have not found an ideal way to balance accurate modeling with generic reuse. Could you suggest what abstraction GCC would prefer in this case, or whether there is a better way to represent these constraint differences between microarchitectures? BR, Jin
