Re: [PATCH v4 2/5] RISC-V: Refine macro-fusion helper predicates

Jeffrey Law <[email protected]> Wed, 5 Aug 2026 17:34:33 -0600
Newsgroups gmane.comp.gcc.patches
Message-ID <[email protected]>

On 7/29/2026 1:17 AM, Jin Ma wrote:
> Factor common RTL matching out of the existing fusion recognizers and
> use it to validate register dependencies consistently.  Restrict ADDI
> matching to immediates encodable by the instruction.
>
> gcc/ChangeLog:
>
> 	* config/riscv/riscv-fusion.cc (riscv_set_extract_word_add_p):
> 	New function.
> 	(riscv_regno): Likewise.
> 	(riscv_fusion_same_dest_p): Handle hard-register SUBREGs and
> 	optionally check the first-source dependency.
> 	(riscv_set_is_addi_p): Check the immediate range.
> 	(riscv_fuse_ldpreincrement): Check the update dependency.
> 	(riscv_fuse_lui_addi): Reuse the ADDI matcher, check the source
> 	dependency, and reject x0 and zero-valued LUI patterns.
> 	(riscv_fuse_auipc_addi): Check the source dependency and reject x0.
> 	(riscv_fuse_auipc_ld): Check the address base dependency.
> 	(riscv_fuse_bfext): Use riscv_regno to validate the source register.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.target/riscv/fusion-lui-addi-boundary.c: New test.
> 	* gcc.target/riscv/fusion-lui-addi-dependency-rtl.c: New test.
> ---
>   gcc/config/riscv/riscv-fusion.cc              | 154 +++++++++++++++---
>   .../riscv/fusion-lui-addi-boundary.c          |  61 +++++++
>   .../riscv/fusion-lui-addi-dependency-rtl.c    | 122 ++++++++++++++
>   3 files changed, 316 insertions(+), 21 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.target/riscv/fusion-lui-addi-boundary.c
>   create mode 100644 gcc/testsuite/gcc.target/riscv/fusion-lui-addi-dependency-rtl.c
>
>
> +static unsigned int
> +riscv_regno (rtx x)
> +{
> +  int regno = true_regnum (x);
> +  if (regno >= 0)
> +    return regno;
> +
> +  /* An unassigned pseudo has no hard-register mapping yet.  Its lowpart can
> +     use the pseudo's identity, but other parts cannot.  */
It's not a big deal IMHO, but you can independently access both SI 
halves of a DI object.  So it's meaningful to ask for the high part as 
well.  It'll eventually map down to a hard reg or stack slot of course.  
  In the before-allocation case these routines are going to over-match, 
it's kind of inherent in the problem and it's OK to over-match at that 
phase.

By being aggressive and overmatching prior to reload we've got a much 
better opportunity to bring fusible instructions together. That 
over-match will be corrected post-reload if the final register selection 
isn't suitable for fusion.  This was one of the key things from 
Artimey's work that made fusion meaningfully more effective which could 
been seen when we evaluated the fusion counts staticly across spec2017 
as well as when running on design where we could see the fusion cases 
reported by the design jump measurably and a small overall gain in 
performance compared to the baseline before Artimey's changes.

It looks like your patch eliminates that behavior.  Prior to register 
allocation I would strongly recommend assuming things match.


> @@ -93,6 +196,7 @@ riscv_set_is_addi_p (rtx set)
>     return (GET_CODE (SET_SRC (set)) == PLUS
>   	  && REG_P (XEXP (SET_SRC (set), 0))
>   	  && CONST_INT_P (XEXP (SET_SRC (set), 1))
> +	  && SMALL_OPERAND (INTVAL (XEXP (SET_SRC (set), 1)))
>   	  && REG_P (SET_DEST (set)));
So I'm curious.  Since we can only see valid insns here, we shouldn't 
see anything except a SMALL_OPERAND here anyway, right? I'm not 
objecting to the check, just want to know what caused you to add it.

My biggest concern here is dropping the assumption that before 
allocation that the registers will match in the preferred way we want.

Jeff