Skip to content

Commit

Permalink
RISC-V: elide unnecessary sign extend when expanding cmp_and_jump
Browse files Browse the repository at this point in the history
RV64 compare and branch instructions only support 64-bit operands.
At Expand time, the backend conservatively zero/sign extends
its operands even if not needed, such as incoming 32-bit function args
which ABI/ISA guarantee to be sign-extended already.

And subsequently REE fails to eliminate them as
   "missing defintion(s)" or "multiple definition(s)
since function args don't have explicit definition.

So during expand riscv_extend_comparands (), if an operand is a
subreg-promoted SI with inner DI, which is representative of a function
arg, just peel away the subreg to expose the DI, eliding the sign
extension. As Jeff noted this routine is also used in if-conversion so
also helps there.

Note there's currently patches floating around to improve REE and also a
new pass to eliminate unneccesary extensions, but it is still beneficial
to not generate those extra extensions in first place. It is obviously
less work for post-reload passes such as REE, but even for earlier
passes, such as combine, having to deal with one less thing and ensuing
fewer combinations is a win too.

Way too many existing tests used to observe this issue.
e.g. gcc.c-torture/compile/20190827-1.c -O2 -march=rv64gc
It elimiates the SEXT.W

Tested with rv64gc with no regressions, I'm relying on PAtrick's
pre-commit CI to do the full testing.

gcc/ChangeLog:
	* config/riscv/riscv.cc (riscv_sign_extend_if_not_subreg_prom): New.
	* (riscv_extend_comparands): Call New function on operands.

Signed-off-by: Vineet Gupta <vineetg@rivosinc.com>
  • Loading branch information
Vineet Gupta authored and ouuleilei-bot committed Oct 30, 2023
1 parent c2d62cd commit 9732245
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions gcc/config/riscv/riscv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3192,6 +3192,24 @@ riscv_zero_if_equal (rtx cmp0, rtx cmp1)
cmp0, cmp1, 0, 0, OPTAB_DIRECT);
}

/* Helper function for riscv_extend_comparands to Sign-extend the OP.
However if the OP is SI subreg promoted with an inner DI, such as
(subreg/s/v:SI (reg/v:DI 150 [ xx ]) 0)
just peel off the SUBREG to get DI, avoiding extraneous extension. */

static void
riscv_sign_extend_if_not_subreg_prom (rtx *op)
{
if (GET_MODE(*op) == SImode
&& GET_CODE (*op) == SUBREG
&& SUBREG_PROMOTED_VAR_P (*op)
&& GET_MODE_SIZE (GET_MODE (XEXP (*op, 0))).to_constant ()
== GET_MODE_SIZE (word_mode))
*op = XEXP (*op, 0);
else
*op = gen_rtx_SIGN_EXTEND (word_mode, *op);
}

/* Sign- or zero-extend OP0 and OP1 for integer comparisons. */

static void
Expand Down Expand Up @@ -3221,9 +3239,10 @@ riscv_extend_comparands (rtx_code code, rtx *op0, rtx *op1)
}
else
{
*op0 = gen_rtx_SIGN_EXTEND (word_mode, *op0);
riscv_sign_extend_if_not_subreg_prom(op0);

if (*op1 != const0_rtx)
*op1 = gen_rtx_SIGN_EXTEND (word_mode, *op1);
riscv_sign_extend_if_not_subreg_prom(op1);
}
}
}
Expand Down

0 comments on commit 9732245

Please sign in to comment.