On 6/13/25 9:01 AM, Umesh Kalappa wrote:
Addressed the most of comments and tried to refactor the
riscv_expand_conditional_move() to some extent.
No regressions are found for "runtest --tool gcc
--target_board='riscv-sim/-mabi=lp64d/-mtune=mips-p8700/-O2 ' riscv.exp"
*config/riscv/riscv-cores.def(RISCV_CORE):Updated the supported march.
*config/riscv/riscv-ext-mips.def(DEFINE_RISCV_EXT):
New file added for mips conditional mov extension.
*config/riscv/riscv-ext.def: Likewise.
*config/riscv/t-riscv:Generates riscv-ext.opt
*config/riscv/riscv-ext.opt: Generated file.
*config/riscv/riscv.cc(riscv_expand_conditional_move):Updated for mips
cmov
and outlined some code that handle arch cond move.
*config/riscv/riscv.md(mov<mode>cc):updated expand for MIPS CCMOV.
*config/riscv/mips-insn.md:New file for mips-p8700 ccmov insn.
*testsuite/gcc.target/riscv/mipscondmov.c:Test file for mips.ccmov insn.
*gcc/doc/riscv-ext.texi:Updated for mips cmov.
---
gcc/config/riscv/mips-insn.md | 37 ++++++
gcc/config/riscv/riscv-cores.def | 3 +-
gcc/config/riscv/riscv-ext-mips.def | 35 +++++
gcc/config/riscv/riscv-ext.def | 1 +
gcc/config/riscv/riscv-ext.opt | 4 +
gcc/config/riscv/riscv.cc | 131 ++++++++++++-------
gcc/config/riscv/riscv.md | 3 +-
gcc/config/riscv/t-riscv | 3 +-
gcc/doc/riscv-ext.texi | 4 +
gcc/testsuite/gcc.target/riscv/mipscondmov.c | 30 +++++
10 files changed, 202 insertions(+), 49 deletions(-)
create mode 100644 gcc/config/riscv/mips-insn.md
create mode 100644 gcc/config/riscv/riscv-ext-mips.def
create mode 100644 gcc/testsuite/gcc.target/riscv/mipscondmov.c
+
+(define_insn "*mov<GPR:mode><X:mode>cc_bitmanip"
+ [(set (match_operand:GPR 0 "register_operand" "=r")
+ (if_then_else:GPR
+ (match_operator 5 "equality_operator"
+ [(match_operand:X 1 "register_operand" "r")
+ (match_operand:X 2 "const_0_operand" "J")])
+ (match_operand:GPR 3 "reg_or_0_operand" "rJ")
+ (match_operand:GPR 4 "reg_or_0_operand" "rJ")))]
+ "TARGET_XMIPSCMOV"
Not something you need to fix to move forward, but more something to
keep in mind.
For the kind of cases in this pattern it may be better to use a code
iterator rather than a match_operator. You can use iterators much like
you could other RTL codes. We already have "any_eq" defined for the
RISC-V port.
And you can trivially get the actual code via a code_attr construct if
you need to.
+
+/* Emit target specific conditional move like TARGET_XMIPSCMOV etc*/
+bool
+riscv_target_conditional_move (rtx dest, rtx op0, rtx op1, rtx_code code,
+ rtx cons, rtx alt)
+{
+ machine_mode dst_mode = GET_MODE (dest);
+ rtx target;
+
+ /* force the operands to the register*/
+ cons = force_reg (dst_mode, cons);
+ alt = force_reg (dst_mode, alt);
+
+ if (TARGET_XMIPSCMOV)
+ {
+ if (code == EQ || code == NE)
+ {
+ op0 = riscv_zero_if_equal (op0, op1);
+ op1 = const0_rtx;
+ }
+ else
+ {
+ target = gen_reg_rtx (GET_MODE (op0));
+ riscv_emit_int_order_test(code, 0, target, op0, op1);
+ op0 = target;
+ op1 = const0_rtx;
+ code = NE;
+ }
I suspect there's something goofy in the indentation in the block above.
Each indention level is 2 spaces and when you get 8 spaces of
indention you should use a tab.
Space before the open paren in the call to riscv_emit_int_order_test.
@@ -5454,34 +5520,22 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx
cons, rtx alt)
@@ -5500,9 +5554,9 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx
cons, rtx alt)
if (code == LE || code == LEU || code == GE || code == GEU)
invert_ptr = &invert;
- /* Emit an SCC-like instruction into a temporary so that we can
- use an EQ/NE comparison. We can support both FP and integer
- conditional moves. */
+ /* Emit an SCC-like instruction into a temporary so that we can
+ use an EQ/NE comparison. We can support both FP and integer
+ conditional moves. */
if (INTEGRAL_MODE_P (mode0))
riscv_expand_int_scc (tmp, code, op0, op1, invert_ptr);
else if (FLOAT_MODE_P (mode0)
Not sure why you changed the indentation here. I just checked the
current version of this code on the trunk and the indentation seems
correct there.
We don't mind indentation/whitespace fixes, but be sure you're actually
fixing something rather than making gratutious changes.
@@ -5558,15 +5599,15 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx
cons, rtx alt)
else if (cons == CONST0_RTX (dst_mode)
&& ((REG_P (alt) || SUBREG_P (alt))
|| (CONST_INT_P (alt) && alt != CONST0_RTX (dst_mode))))
- {
- riscv_emit_int_compare (&code, &op0, &op1, true);
- rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
- alt = force_reg (dst_mode, alt);
- emit_insn (gen_rtx_SET (dest,
- gen_rtx_IF_THEN_ELSE (dst_mode, cond,
- cons, alt)));
- return true;
- }
+ {
+ riscv_emit_int_compare (&code, &op0, &op1, true);
+ rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+ alt = force_reg (dst_mode, alt);
+ emit_insn (gen_rtx_SET (dest,
+ gen_rtx_IF_THEN_ELSE (dst_mode, cond,
+ cons, alt)));
+ return true;
+ }
Again, not sure why you're changing the indentation here.
Generally the implementation looks good. You just need to eliminate the
gratutious changes and fix the indentation/whitespace in your new code.
I think it's basically ready to go with those fixes.
Jeff
