On 5/27/25 5:06 AM, Umesh Kalappa wrote:
The P8700 is a high-performance processor from MIPS by extending RISCV with
the MIPS custom instruction and the following changes are added to enable the
conditional move support from mips
No regressions are found for "runtest --tool gcc
--target_board='riscv-sim/-mabi=lp64d/-mcmodel=medlow/-mtune=mips-p8700/-O2 '
riscv.exp"
*config/riscv/riscv-cores.def(RISCV_CORE):Updated the march for
mips-p8700 tune.
*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.
*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.
---
diff --git a/gcc/config/riscv/mips-insn.md b/gcc/config/riscv/mips-insn.md
new file mode 100644
index 00000000000..ee106c4221e
--- /dev/null
+++ b/gcc/config/riscv/mips-insn.md
@@ -0,0 +1,37 @@
+;; Machine description for MIPS custom instructioins.
Typo. Should be "instructions".
diff --git a/gcc/config/riscv/riscv-cores.def b/gcc/config/riscv/riscv-cores.def
index 118fef23cad..b8bf81e7883 100644
--- a/gcc/config/riscv/riscv-cores.def
+++ b/gcc/config/riscv/riscv-cores.def
@@ -154,7 +154,7 @@ RISCV_CORE("xiangshan-nanhu",
"rv64imafdc_zba_zbb_zbc_zbs_"
"svinval_zicbom_zicboz",
"xiangshan-nanhu")
-RISCV_CORE("mips-p8700", "rv64imafd_zicsr_zmmul_"
+RISCV_CORE("mips-p8700", "rv64imafd_"
"zaamo_zalrsc_zba_zbb",
"mips-p8700")
You know your core better than I, so I'm going to assume your
architecture string is reasonably correct. But the change does not
match your ChangeLog entry. You're not adjusting tuning at all. This
adjusts the supported architecture.
+ /* UPPERCAE_NAME */ XMIPSCMOV,
Should be "UPPERCASE". I think Kito got it wrong in his original patch
and the typo keeps getting copied into new files ;(
@@ -5352,8 +5352,31 @@ riscv_expand_conditional_move (rtx dest, rtx op, rtx
cons, rtx alt)
rtx_code code = GET_CODE (op);
rtx op0 = XEXP (op, 0);
rtx op1 = XEXP (op, 1);
+ rtx target;
- if (((TARGET_ZICOND_LIKE
+ if (TARGET_XMIPSCMOV && mode == word_mode && GET_MODE (op0) == word_mode)
+ {
+ if (code == EQ || code == NE)
+ {
+ op0 = riscv_zero_if_equal (op0, op1);
+ op1 = const0_rtx;
+ }
Your formatting looks goofy here. The open-curley appears to be
indented one space inside the IF. Occasionally this is due to tabs, so
double check it may be a false positive.
+ else
+ {
+ target = gen_reg_rtx (GET_MODE (op0));
+ riscv_emit_int_order_test(code, 0, target, op0, op1);
Always a space between the function name and the open paren for the
argument list.
Given your semantics are full conditional move rather than zicond I
wonder if we should restructure this code a little. Other than things
like canonicalization of the condition I doubt we're going to be able to
share much between the zicond forms and full conditional move forms.
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 92fe7c7741a..0c25a723d9c 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -3290,8 +3290,18 @@
(match_operand:GPR 2 "movcc_operand")
(match_operand:GPR 3 "movcc_operand")))]
"TARGET_SFB_ALU || TARGET_XTHEADCONDMOV || TARGET_ZICOND_LIKE
- || TARGET_MOVCC"
+ || TARGET_MOVCC || TARGET_XMIPSCMOV"
{
+ if (TARGET_XMIPSCMOV) {
+ /* operands[2] must be register or 0. */
+ if (!reg_or_0_operand (operands[2], GET_MODE (operands[2])))
+ operands[2] = force_reg (<GPR:MODE>mode, operands[2]);
+
+ /* operands[3] must be register or 0. */
+ if (!reg_or_0_operand (operands[3], GET_MODE (operands[3])))
+ operands[3] = force_reg (<GPR:MODE>mode, operands[3]);
+ }
Can this be pushed into riscv_expand_conditional_move? Essentially with
your patch we have two places where operands are adjusted. We should
avoid that if we easily can.
Overall it looks pretty sensible. My biggest concern is overall
structure of riscv_expand_conditional_move. I kind of get the sense
that we need to refactor operand canonicalization into its own routine,
then have two subroutines, one for the zicond like variants and another
for the full conditional move rather than trying to handle them both
inside riscv_expand_conditional_move given I don't expect them to share
much code.
Thoughts?
jeff
