diff mbox series

[22/44] RISC-V: Fold all the cond-move variants together

Message ID alpine.DEB.2.20.2311181813500.5892@tpp.orcam.me.uk
State New
Headers show
Series RISC-V: Various if-conversion fixes and improvements | expand

Commit Message

Maciej W. Rozycki Nov. 19, 2023, 5:40 a.m. UTC 
Code in `riscv_expand_conditional_move' for Ventana and Zicond targets 
seems like bolted on as an afterthought rather than properly merged so 
as to handle all the cases together.

Fold the existing code pieces together then (observing that for short 
forward branch targets no integer comparisons need to be canonicalized), 
letting T-Head targets produce branchless sequences for all the integer 
comparisons rather than for equality ones only, and preparing for the 
handling of floating-point comparisons here across all conditional-move 
targets.

	gcc/
	* config/riscv/riscv.cc (riscv_expand_conditional_move): Unify
	conditional-move handling across all the relevant targets.
---
 gcc/config/riscv/riscv.cc |   58 +++++++++++++++++++---------------------------
 1 file changed, 25 insertions(+), 33 deletions(-)

gcc-riscv-expand-conditional-move-sfb-alu-thead.diff

Comments

Jeff Law Nov. 19, 2023, 6:35 p.m. UTC | #1 
On 11/18/23 22:40, Maciej W. Rozycki wrote:
> Code in `riscv_expand_conditional_move' for Ventana and Zicond targets
> seems like bolted on as an afterthought rather than properly merged so
> as to handle all the cases together.
You could characterize it that way.  It was mostly a desire to not muck 
up any of the thead or sfb code which I know next to nothing about.  I 
think with the improvements in the testsuite from your series it's a lot 
more feasible to unify the implementations and be sure we haven't broken 
something along the way.


> 
> Fold the existing code pieces together then (observing that for short
> forward branch targets no integer comparisons need to be canonicalized),
> letting T-Head targets produce branchless sequences for all the integer
> comparisons rather than for equality ones only, and preparing for the
> handling of floating-point comparisons here across all conditional-move
> targets.
> 
> 	gcc/
> 	* config/riscv/riscv.cc (riscv_expand_conditional_move): Unify
> 	conditional-move handling across all the relevant targets.
OK

jeff
diff mbox series

Patch

Index: gcc/gcc/config/riscv/riscv.cc
===================================================================
--- gcc.orig/gcc/config/riscv/riscv.cc
+++ gcc/gcc/config/riscv/riscv.cc
@@ -4090,35 +4090,9 @@  riscv_expand_conditional_move (rtx dest,
   rtx_code code = GET_CODE (op);
   rtx op0 = XEXP (op, 0);
   rtx op1 = XEXP (op, 1);
-  bool need_eq_ne_p = false;
-
-  if (TARGET_XTHEADCONDMOV
-      && GET_MODE_CLASS (mode) == MODE_INT
-      && (GET_MODE (op) == mode || GET_MODE (op) == E_VOIDmode)
-      && (GET_MODE (op0) == mode || CONST_INT_P (op0))
-      && (GET_MODE (op1) == mode || CONST_INT_P (op1))
-      && (code == EQ || code == NE))
-    need_eq_ne_p = true;
-
-  if (need_eq_ne_p
-      || (TARGET_SFB_ALU && GET_MODE (op0) == word_mode))
-    {
-      riscv_emit_int_compare (&code, &op0, &op1, need_eq_ne_p);
-      rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
 
-      /* The expander is a bit loose in its specification of the true
-	 arm of the conditional move.  That allows us to support more
-	 cases for extensions which are more general than SFB.  But
-	 does mean we need to force CONS into a register at this point.  */
-      cons = force_reg (mode, cons);
-      /* With XTheadCondMov we need to force ALT into a register too.  */
-      alt = force_reg (mode, alt);
-      emit_insn (gen_rtx_SET (dest, gen_rtx_IF_THEN_ELSE (mode,
-							  cond, cons, alt)));
-      return true;
-    }
-  else if (TARGET_ZICOND_LIKE
-	   && GET_MODE_CLASS (mode) == MODE_INT)
+  if ((TARGET_ZICOND_LIKE && GET_MODE_CLASS (mode) == MODE_INT)
+      || TARGET_SFB_ALU || TARGET_XTHEADCONDMOV)
     {
       machine_mode mode0 = GET_MODE (op0);
       machine_mode mode1 = GET_MODE (op1);
@@ -4132,9 +4106,11 @@  riscv_expand_conditional_move (rtx dest,
 	return false;
 
       /* Canonicalize the comparison.  It must be an equality comparison
-	 of integer operands.  If it isn't, then emit an SCC instruction
+	 of integer operands, or with SFB it can be any comparison of
+	 integer operands.  If it isn't, then emit an SCC instruction
 	 so that we can then use an equality comparison against zero.  */
-      if (!equality_operator (op, VOIDmode) || !INTEGRAL_MODE_P (mode0))
+      if ((!TARGET_SFB_ALU && !equality_operator (op, VOIDmode))
+	  || !INTEGRAL_MODE_P (mode0))
 	{
 	  bool *invert_ptr = nullptr;
 	  bool invert = false;
@@ -4166,10 +4142,26 @@  riscv_expand_conditional_move (rtx dest,
 	  op1 = XEXP (op, 1);
 	}
 
+      if (TARGET_SFB_ALU || TARGET_XTHEADCONDMOV)
+	{
+	  riscv_emit_int_compare (&code, &op0, &op1, !TARGET_SFB_ALU);
+	  rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);
+
+	  /* The expander is a bit loose in its specification of the true
+	     arm of the conditional move.  That allows us to support more
+	     cases for extensions which are more general than SFB.  But
+	     does mean we need to force CONS into a register at this point.  */
+	  cons = force_reg (mode, cons);
+	  /* With XTheadCondMov we need to force ALT into a register too.  */
+	  alt = force_reg (mode, alt);
+	  emit_insn (gen_rtx_SET (dest, gen_rtx_IF_THEN_ELSE (mode, cond,
+							      cons, alt)));
+	  return true;
+	}
       /* 0, reg or 0, imm */
-      if (cons == CONST0_RTX (mode)
-	  && (REG_P (alt)
-	      || (CONST_INT_P (alt) && alt != CONST0_RTX (mode))))
+      else if (cons == CONST0_RTX (mode)
+	       && (REG_P (alt)
+		   || (CONST_INT_P (alt) && alt != CONST0_RTX (mode))))
 	{
 	  riscv_emit_int_compare (&code, &op0, &op1, true);
 	  rtx cond = gen_rtx_fmt_ee (code, GET_MODE (op0), op0, op1);