diff mbox series

[committed,RISC-V] Fix nearbyint failure on rv32 and formatting nits

Message ID f12a2c22-aa93-4579-a8c9-fcf2edd07fcf@ventanamicro.com
State New
Headers show
Series [committed,RISC-V] Fix nearbyint failure on rv32 and formatting nits | expand

Commit Message

Jeff Law May 2, 2024, 11:16 p.m. UTC 
The CI system tripped an execution failure for rv32 with the ceil/round 
patch.

The fundamental problem is the FP->INT step in these sequences requires 
the input size to match the output size.  The output size was based on 
rv32/rv64.  Meaning that we'd try to do DF->SI->DF.

That doesn't preserve the semantics we want in at least two ways.

The net is we can't use this trick for DF values on rv32.  While inside 
the code I realized we had a similar problem for HF modes.  HF modes we 
can support only for Zfa.  So I fixed that proactively.

The CI system also pointed out various formatting nits.  I think this 
fixes all but one overly long line.

Note I could have factored the TARGET_ZFA test.  But I think as-written 
it's clearer what the desired cases to transform are.

Tested on rv32gcv which verified the failing test passes again.

Pushed to the trunk.

Jeff
commit 8367c996e55b2c54aeee25e446357a1015a1d11d
Author: Jeff Law <jlaw@ventanamicro.com>
Date:   Thu May 2 17:13:12 2024 -0600

    [committed][RISC-V] Fix nearbyint failure on rv32 and formatting nits
    
    The CI system tripped an execution failure for rv32 with the ceil/round patch.
    
    The fundamental problem is the FP->INT step in these sequences requires the
    input size to match the output size.  The output size was based on rv32/rv64.
    Meaning that we'd try to do DF->SI->DF.
    
    That doesn't preserve the semantics we want in at least two ways.
    
    The net is we can't use this trick for DF values on rv32.  While inside the
    code I realized we had a similar problem for HF modes.  HF modes we can support
    only for Zfa.  So I fixed that proactively.
    
    The CI system also pointed out various formatting nits.  I think this fixes all
    but one overly long line.
    
    Note I could have factored the TARGET_ZFA test.  But I think as-written it's
    clearer what the desired cases to transform are.
    
    gcc/
            * config/riscv/riscv.md (<round_pattern><ANYF:mode>2): Adjust
            condition to match what can be properly implemented.  Fix various
            formatting issues.
            (l<round_pattern><ANYF:mode>si2_sext): Fix formatting
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index b9b0acf92c7..d4676507b45 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -2077,8 +2077,8 @@  (define_insn "*l<round_pattern><ANYF:mode>si2"
 (define_insn "l<round_pattern><ANYF:mode>si2_sext"
   [(set (match_operand:DI       0 "register_operand" "=r")
 	 (sign_extend:DI (unspec:SI
-	                     [(match_operand:ANYF 1 "register_operand" " f")]
-                      ROUND)))]
+			     [(match_operand:ANYF 1 "register_operand" " f")]
+		      ROUND)))]
   "TARGET_64BIT && (TARGET_HARD_FLOAT || TARGET_ZFINX)"
   "fcvt.w.<ANYF:fmt> %0,%1,<round_rm>"
   [(set_attr "type" "fcvt_f2i")
@@ -2094,13 +2094,25 @@  (define_insn "l<round_pattern><ANYF:mode>di2"
   [(set_attr "type" "fcvt_f2i")
    (set_attr "mode" "<ANYF:MODE>")])
 
+;; There are a couple non-obvious restrictions to be aware of.
+;;
+;; We'll do a FP-INT conversion in the sequence.  But we don't
+;; have a .l (64bit) variant of those instructions for rv32.
+;; To preserve proper semantics we must reject DFmode inputs
+;; for rv32 unless Zfa is enabled.
+;;
+;; The ANYF iterator allows HFmode.  We don't have all the
+;; necessary patterns defined for HFmode.  So restrict HFmode
+;; to TARGET_ZFA.
 (define_expand "<round_pattern><ANYF:mode>2"
   [(set (match_operand:ANYF     0 "register_operand" "=f")
-        (unspec:ANYF
-            [(match_operand:ANYF 1 "register_operand" " f")]
-        ROUND))]
-  "TARGET_HARD_FLOAT && (TARGET_ZFA
-                         || flag_fp_int_builtin_inexact || !flag_trapping_math)"
+	(unspec:ANYF
+	    [(match_operand:ANYF 1 "register_operand" " f")]
+	ROUND))]
+  "(TARGET_HARD_FLOAT
+    && (TARGET_ZFA || flag_fp_int_builtin_inexact || !flag_trapping_math)
+    && (TARGET_ZFA || TARGET_64BIT || <ANYF:MODE>mode != DFmode)
+    && (TARGET_ZFA || <ANYF:MODE>mode != HFmode))"
 {
   if (TARGET_ZFA)
     emit_insn (gen_<round_pattern><ANYF:mode>_zfa2 (operands[0],
@@ -2116,7 +2128,7 @@  (define_expand "<round_pattern><ANYF:mode>2"
 
       riscv_emit_move (tmp_reg, operands[1]);
       riscv_emit_move (coeff_reg,
-                       riscv_vector::get_fp_rounding_coefficient (<ANYF:MODE>mode));
+		       riscv_vector::get_fp_rounding_coefficient (<ANYF:MODE>mode));
       emit_insn (gen_abs<ANYF:mode>2 (abs_reg, operands[1]));
 
       riscv_expand_conditional_branch (label, LT, abs_reg, coeff_reg);
@@ -2126,29 +2138,20 @@  (define_expand "<round_pattern><ANYF:mode>2"
 
       emit_label (label);
       switch (<ANYF:MODE>mode)
-        {
-        case SFmode:
-          reg = gen_reg_rtx (SImode);
-          emit_insn (gen_l<round_pattern>sfsi2 (reg, operands[1]));
-          emit_insn (gen_floatsisf2 (abs_reg, reg));
-          break;
-        case DFmode:
-          if (TARGET_64BIT)
-            {
-              reg = gen_reg_rtx (DImode);
-              emit_insn (gen_l<round_pattern>dfdi2 (reg, operands[1]));
-              emit_insn (gen_floatdidf2 (abs_reg, reg));
-            }
-          else
-            {
-              reg = gen_reg_rtx (SImode);
-              emit_insn (gen_l<round_pattern>dfsi2 (reg, operands[1]));
-              emit_insn (gen_floatsidf2 (abs_reg, reg));
-            }
-          break;
-        default:
-          gcc_unreachable ();
-        }
+	{
+	case SFmode:
+	  reg = gen_reg_rtx (SImode);
+	  emit_insn (gen_l<round_pattern>sfsi2 (reg, operands[1]));
+	  emit_insn (gen_floatsisf2 (abs_reg, reg));
+	  break;
+	case DFmode:
+	  reg = gen_reg_rtx (DImode);
+	  emit_insn (gen_l<round_pattern>dfdi2 (reg, operands[1]));
+	  emit_insn (gen_floatdidf2 (abs_reg, reg));
+	  break;
+	default:
+	  gcc_unreachable ();
+	}
 
       emit_insn (gen_copysign<ANYF:mode>3 (tmp_reg, abs_reg, operands[1]));