On 1/9/26 02:16, Max Chou wrote:
+bool float8_e4m3_is_quiet_nan(float8_e4m3 a_, float_status *status)
+{
+    return float8_e4m3_is_any_nan(a_);
+}
+
+bool float8_e5m2_is_quiet_nan(float8_e5m2 a_, float_status *status)
+{
+    if (no_signaling_nans(status) || status->ocp_fp8e5m2_no_signal_nan) {

What is this new thing?

+        return float8_e5m2_is_any_nan(a_);
+    } else {
+        uint8_t a = float8_e5m2_val(a_);
+        if (snan_bit_is_one(status)) {
+            return (((a >> 1) & 0x3F) == 0x3E) && (a & 0x1);
+        } else {
+            return ((a >> 1) & 0x3F) == 0x3F;
+        }
+    }
+}
...
+bool float8_e4m3_is_signaling_nan(float8_e4m3 a_, float_status *status)
+{
+    if (no_signaling_nans(status)) {
+        return false;
+    } else {
+        if (snan_bit_is_one(status)) {
+            return float8_e4m3_is_any_nan(a_);
+        } else {
+            return false;
+        }
+    }
+}
+
+bool float8_e5m2_is_signaling_nan(float8_e5m2 a_, float_status *status)
+{
+    if (no_signaling_nans(status)) {

... which is not also reflected here?

+        return false;
+    } else {
+        uint8_t a = float8_e5m2_val(a_);
+        if (snan_bit_is_one(status)) {
+            return ((a >> 1) & 0x3F) == 0x3F;
+        } else {
+            return (((a >> 1) & 0x3F) == 0x3E && (a & 0x1));
+        }
+    }
+}

(0) We really should clean up this code so that there's not so much duplication.
FOO_is_quiet_nan and FOO_is_signaling_nan really should share code.
That would have caught the above.

(1) RISC-V always uses default nan mode, the OCP spec declines to define SNaN vs QNaN, leaving the 8 unique NaN encodings unspecified, and RISC-V does not do so either. You assert later:

+     * RISC-V uses only quiet NaNs in its OCP FP8 implementation.

Is this out-of-band discussion with engineers?
Because it's missing from the (remarkably short) document.

(2) Arm does specify (see FP8Unpack in the ARM pseudocode), doing the usual thing in taking the msb of the mantissa for SNaN. Which means that E4M3 is *always* SNaN.

Both architectures then immediately convert to FP16 default nan, however Arm *does* raise invalid operand exception for the SNaN, so we can't just ignore it.

Given that there's exactly one RISC-V instruction for which this matters, vfwcvtbf16.f.f.v, it seems like it might be better to simply adjust float_status.no_signaling_nans within the helper rather than introduce ocp_fp8e5m2_no_signal_nan.

+    /*
+     * When true, OCP FP8 formats use the same canonical NaN representation
+     * (0x7F) for all NaN outputs. RISC-V specifies a single canonical NaN
+     * for both E4M3 and E5M2.
+     */
+    bool ocp_fp8_same_canonical_nan;

Similarly you could adjust the canonical nan around the 4 FP16->FP8 conversion 
insn helpers:

    /* Default NaN value: sign bit clear, all frac bits set */
    set_float_default_nan_pattern(0b01111111, &env->fp_status);

In either case, "bool" doesn't seem appropriate.

FWIW, Arm retains the msb set pattern as for all other fp formats 
(FP8DefaultNaN).


r~

Reply via email to