Re: [PATCH v4 4/5] RISC-V: Add macro-fusion pair recognition

On 8/6/2026 9:11 AM, Jin Ma wrote:
> Hi Jeff,
>
>> On 7/29/2026 1:17 AM, Jin Ma wrote:
>>> Add recognition for the following macro-fusion pairs:
>>>
>>>     RISCV_FUSE_SUB_SEQZ        - sub/subw + seqz/snez
>>>     RISCV_FUSE_ADD_LD          - add type + integer load
>>>     RISCV_FUSE_ADD_ST          - add type + integer store
>>>     RISCV_FUSE_ADD_ANDI        - add/addi type + andi type
>>>     RISCV_FUSE_ANDI_ADD        - andi type + add/addi type
>>>     RISCV_FUSE_LOGIC_LOGIC     - logic + logic
>>>     RISCV_FUSE_SLLI_SRLI       - slli/slliw + srli/srliw
>>>     RISCV_FUSE_SRLI_ADD        - srli/srliw + add type
>>>     RISCV_FUSE_PREINDEX_LD     - addi type + load
>>>     RISCV_FUSE_PREINDEX_ST     - addi type + store
>>>     RISCV_FUSE_POSTINDEX_LD    - load + addi type
>>>     RISCV_FUSE_POSTINDEX_ST    - store + addi type
>>>     RISCV_FUSE_LDST_PAIR_INC   - ascending integer load/store pair
>>>     RISCV_FUSE_LDST_PAIR_DEC   - descending integer load/store pair
>>>     RISCV_FUSE_FLDFST_PAIR_INC - ascending FP load/store pair
>>>     RISCV_FUSE_FLDFST_PAIR_DEC - descending FP load/store pair
>>>
>>> Leave the pairs disabled by default and mark positive checks as XFAIL.
>>>
>>> gcc/ChangeLog:
>>>
>>>   * config/riscv/riscv-fusion.cc: Include insn-attr.h.
>>>   (riscv_fusion_same_source_p, riscv_set_is_addw_p,
>>>   riscv_set_is_word_add_p, riscv_set_is_addiw_p, riscv_insn_is_mv_li_p,
>>>   riscv_insn_is_addi_type_p, riscv_insn_is_nonword_addi_type_p,
>>>   riscv_insn_is_add_type_p, riscv_insn_is_add_addi_p,
>>>   riscv_insn_is_sub_type_p, riscv_insn_is_andi_type_p,
>>>   riscv_insn_is_logical_type_p, riscv_set_is_slli_p,
>>>   riscv_set_is_slliw_p, riscv_set_is_srli_p, riscv_set_is_srliw_p,
>>>   riscv_set_is_word_shift_p, riscv_fusion_extract_address,
>>>   riscv_fusion_load_store, riscv_insn_is_load_store_type_p): New
>>>   helper functions.
>>>   (riscv_fuse_sub_seqz, riscv_fuse_add_andi, riscv_fuse_add_ldst_1,
>>>   riscv_fuse_add_ld, riscv_fuse_add_st, riscv_fuse_andi_add,
>>>   riscv_fuse_logic_logic, riscv_fuse_slli_srli, riscv_fuse_srli_add,
>>>   riscv_fuse_preindex_ldst_1, riscv_fuse_preindex_ld,
>>>   riscv_fuse_preindex_st, riscv_fuse_postindex_ldst_1,
>>>   riscv_fuse_postindex_ld, riscv_fuse_postindex_st,
>>>   riscv_fuse_ldst_pair_1, riscv_fuse_ldst_pair_inc,
>>>   riscv_fuse_ldst_pair_dec, riscv_fuse_fldfst_pair_inc,
>>>   riscv_fuse_fldfst_pair_dec): New function implementing the
>>>   corresponding RISCV_FUSE_* pair.
>>>   (riscv_fusion_table): Add entries for the new pairs.
>>>   * config/riscv/riscv-protos.h (enum riscv_fusion_pairs): Add pair
>>>   flags.
>> This is getting pretty large and tedious to try and work through from a
>> review standpoint.  I hate to ask, but can we break this down into
>> individual cases to aid review?
>>
>> jeff
> Yes, certainly.  I am sorry that this patch made the review harder.  I
> had assumed that fewer patches would reduce the overall review work,
> but overlooked that putting all the recognizers in one patch makes each
> individual case harder to review.
>
> I will split the patch by individual fusion case or closely related
> fusion family, and place the necessary common helpers before the
> relevant features.  This should make each patch independently
> understandable and reviewable.  I apologize again for the extra work.
No need to apologize.  It's often not obvious when to break something 
down or leave it as is.   Thanks for being willing to break it down, I 
do realize it's more work for you, but I suspect we'll be able to move a 
lot of these forward quickly and we may end up iterating on one or two.

Jeff