fix(codegen): Buffer.readFloatLE does not compile on main — give fcmp an operand type (#10779 follow-up)

changelog.d/10779-fcmp-operand-type.md

@@ -0,0 +1,7 @@
+**`Buffer.readFloatLE` and `readFloatBE` compile again.**
+
+The NaN canonicaliser added for #10779 emits an `fcmp` on an `f32` lane, but `LlBlock::fcmp` rendered its operand type as `double` unconditionally and the in-process LLVM builder hardcoded the same. Any module calling `Buffer.readFloatLE` or `readFloatBE` therefore failed to compile with `'%r1' defined with type 'float' but expected 'double'`.
+
+`LlInst::FCmp` now carries its operand type. `fcmp()` keeps its `double` signature and delegates to a new `fcmp_ty()`, so no existing call site changes.
+
+Also closes the remaining raw float source: NaNs arriving from native code. A C `float` return previously segfaulted — the forged `StringHeader*` was dereferenced — and a C `double` return printed its payload integer. The `double` arm is gated strictly on the manifest declaring `F64`, because it also serves perry's own double ABI where the value already *is* a NaN box.

crates/perry-codegen/src/block.rs

@@ -561,10 +561,18 @@ impl LlBlock {
     /// Float comparison. `cond` is an LLVM predicate string: `olt`, `ole`,
     /// `ogt`, `oge`, `oeq`, `one`, `ord`, `uno`, …
     pub fn fcmp(&mut self, cond: &str, a: &str, b: &str) -> String {
+        self.fcmp_ty(crate::types::DOUBLE, cond, a, b)
+    }
+
+    /// `fcmp` on an operand type other than `double` — a `float` in the
+    /// native lattice, above all. The untyped [`Self::fcmp`] above assumes
+    /// `double`; calling it on a `float` emits IR LLVM rejects.
+    pub fn fcmp_ty(&mut self, ty: LlvmType, cond: &str, a: &str, b: &str) -> String {
         let r = self.reg();
         self.push_inst(crate::inst::LlInst::FCmp {
             dst: r.clone(),
             pred: cond.to_string(),
+            ty,
             a: a.to_string(),
             b: b.to_string(),
         });

crates/perry-codegen/src/dialect/mod.rs

@@ -1352,8 +1352,11 @@ impl<'ctx, 'm> FnReader<'ctx, 'm> {
                 }
                 self.def(dst, out)
             }
-            I::FCmp { dst, pred, a, b } => {
-                let t = basic_type(self.ctx, "double")?;
+            // #10779: `ty` was hardcoded `"double"` here and in `inst.rs`.
+            // Kept on one line: this file sits against the 2000-line gate.
+            #[rustfmt::skip]
+            I::FCmp { dst, pred, ty, a, b } => {
+                let t = basic_type(self.ctx, ty)?;
                 let av = self.val(t, a)?;
                 let bv = self.val(t, b)?;
                 let out: BasicValueEnum = self

crates/perry-codegen/src/expr/nanbox_inline.rs

@@ -26,8 +26,9 @@ pub(crate) fn nanbox_canon_enabled() -> bool {
     })
 }
 
-/// #10779: collapse any NaN in a float lane just loaded from ArrayBuffer-backed
-/// memory to the canonical quiet NaN, so it cannot alias a NaN-box tag.
+/// #10779: collapse any NaN in a raw native `f64` — an ArrayBuffer float lane,
+/// a POD record field, or a C function's `double` return — to the canonical
+/// quiet NaN, so it cannot alias a NaN-box tag.
 ///
 /// The runtime twin is `perry_runtime::array::canonical_raw_f64`, whose doc
 /// comment carries the full argument for why EVERY NaN must be collapsed and
@@ -64,7 +65,10 @@ pub(crate) fn canonicalize_lane_f32(blk: &mut LlBlock, value: &str) -> String {
     if !nanbox_canon_enabled() {
         return value.to_string();
     }
-    let is_nan = blk.fcmp("uno", value, value);
+    // MUST be `fcmp uno float`, not the `double` default: the operand is an
+    // f32 in the native lattice. Emitting `double` here made every program
+    // calling `Buffer.readFloatLE` fail codegen.
+    let is_nan = blk.fcmp_ty(F32, "uno", value, value);
     blk.select(I1, &is_nan, F32, CANONICAL_QNAN_DOUBLE, value)
 }
 
@@ -118,3 +122,93 @@ pub(crate) fn i32_to_nanbox(blk: &mut LlBlock, i32_val: &str) -> String {
     let tagged = blk.or(I64, &payload, INT32_TAG_I64);
     blk.bitcast_i64_to_double(&tagged)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::block::{LlBlock, RegCounter};
+    use crate::inst::LlInst;
+    use crate::types::DOUBLE;
+    use std::rc::Rc;
+
+    fn blk() -> LlBlock {
+        LlBlock::new("t", Rc::new(RegCounter::new()))
+    }
+
+    /// #10779 follow-up. `LlBlock::fcmp` renders its operand type as `double`
+    /// unconditionally, so canonicalising an f32 lane through it emitted
+    /// `fcmp uno double %f32` — IR LLVM rejects with
+    /// "'%r' defined with type 'float' but expected 'double'". Every program
+    /// calling `Buffer.readFloatLE` failed codegen, and no fixture in the
+    /// suite called it, so nothing caught it.
+    ///
+    /// The sabotage is one word: change `F32` back to `DOUBLE` in
+    /// `canonicalize_lane_f32` and this test fails, naming the emitted type.
+    #[test]
+    fn f32_canonicalisation_compares_as_float_not_double() {
+        let mut b = blk();
+        let out = canonicalize_lane_f32(&mut b, "%x");
+        assert_ne!(out, "%x", "the f32 lane must actually be canonicalised");
+        let fcmp = b
+            .insts()
+            .iter()
+            .find_map(|i| match i {
+                LlInst::FCmp { pred, ty, .. } => Some((pred.clone(), *ty)),
+                _ => None,
+            })
+            .expect("canonicalize_lane_f32 must emit an fcmp");
+        assert_eq!(fcmp.0, "uno", "the NaN test must be an unordered compare");
+        assert_eq!(
+            fcmp.1, F32,
+            "an f32 lane must be compared AS float; `double` here is IR LLVM \
+             rejects and it broke every `Buffer.readFloatLE` call site"
+        );
+    }
+
+    /// The f64 twin, so a future edit cannot fix the f32 case by widening
+    /// both to `float`.
+    #[test]
+    fn f64_canonicalisation_compares_as_double() {
+        let mut b = blk();
+        let out = canonicalize_lane_f64(&mut b, "%x");
+        assert_ne!(out, "%x");
+        let ty = b
+            .insts()
+            .iter()
+            .find_map(|i| match i {
+                LlInst::FCmp { ty, .. } => Some(*ty),
+                _ => None,
+            })
+            .expect("canonicalize_lane_f64 must emit an fcmp");
+        assert_eq!(ty, DOUBLE);
+    }
+
+    /// Both canonicalisers must select the SAME canonical quiet NaN, spelled
+    /// in LLVM's hex double form so the payload cannot be rounded away, and
+    /// must select it on the TRUE (is-NaN) arm.
+    #[test]
+    fn both_select_the_canonical_quiet_nan_on_the_nan_arm() {
+        for (name, want_ty) in [("f64", DOUBLE), ("f32", F32)] {
+            let mut b = blk();
+            if name == "f64" {
+                let _ = canonicalize_lane_f64(&mut b, "%x");
+            } else {
+                let _ = canonicalize_lane_f32(&mut b, "%x");
+            }
+            let sel = b
+                .insts()
+                .iter()
+                .find_map(|i| match i {
+                    LlInst::Select { ty, a, b: fb, .. } => Some((*ty, a.clone(), fb.clone())),
+                    _ => None,
+                })
+                .unwrap_or_else(|| panic!("{name} must emit a select"));
+            assert_eq!(sel.0, want_ty, "{name} select operand type");
+            assert_eq!(
+                sel.1, CANONICAL_QNAN_DOUBLE,
+                "{name} must pick the canonical quiet NaN when the value IS a NaN"
+            );
+            assert_eq!(sel.2, "%x", "{name} must pass a non-NaN through unchanged");
+        }
+    }
+}

crates/perry-codegen/src/expr/pod_record.rs

@@ -480,6 +480,15 @@ pub(crate) fn load_pod_field_native(
     let llvm_ty =
         llvm_type_for_native_rep(&field.native_rep).expect("pod field reps have scalar LLVM types");
     let value = ctx.block().load_aligned(llvm_ty, &ptr, field.alignment);
+    // #10779: a POD record's backing memory is a native struct — written by C,
+    // by Rust, or by a previous native store — so its float fields can hold any
+    // NaN, including one whose bits alias a NaN-box tag. The integer field reps
+    // cannot be NaN and pay nothing.
+    let value = match field.native_rep {
+        NativeRep::F64 => crate::expr::nanbox_inline::canonicalize_lane_f64(ctx.block(), &value),
+        NativeRep::F32 => crate::expr::nanbox_inline::canonicalize_lane_f32(ctx.block(), &value),
+        _ => value,
+    };
     let lowered = LoweredValue {
         semantic: SemanticKind::JsNumber,
         rep: field.native_rep.clone(),

crates/perry-codegen/src/inst.rs

@@ -69,6 +69,11 @@ pub enum LlInst {
     FCmp {
         dst: String,
         pred: String,
+        /// Operand type. This USED to be hardcoded `double` at the render
+        /// site, which silently produced invalid IR for a `float` operand
+        /// (#10779 follow-up: `Buffer.readFloatLE` failed codegen with
+        /// "'%r' defined with type 'float' but expected 'double'").
+        ty: LlvmType,
         a: String,
         b: String,
     },
@@ -193,8 +198,14 @@ impl LlInst {
             LlInst::FNeg { dst, pre, a } => {
                 let _ = write!(out, "  {dst} = fneg {pre}double {a}");
             }
-            LlInst::FCmp { dst, pred, a, b } => {
-                let _ = write!(out, "  {dst} = fcmp {pred} double {a}, {b}");
+            LlInst::FCmp {
+                dst,
+                pred,
+                ty,
+                a,
+                b,
+            } => {
+                let _ = write!(out, "  {dst} = fcmp {pred} {ty} {a}, {b}");
             }
             LlInst::ICmp {
                 dst,

crates/perry-codegen/src/lower_call/extern_func.rs

@@ -1769,6 +1769,10 @@ pub fn try_lower_extern_func_call(
         } else if returns_f32 {
             ctx.pending_declares.push((name.clone(), F32, arg_types));
             let raw = ctx.block().call(F32, name, &arg_slices);
+            // #10779: a C `float` return is raw native bits; an f32 NaN
+            // widens KEEPING its payload (0x7FFFFFFF -> a forged
+            // StringHeader*). Canonicalise before `materialize_js_value`.
+            let raw = crate::expr::nanbox_inline::canonicalize_lane_f32(ctx.block(), &raw);
             let lowered = LoweredValue::f32(raw.clone());
             if let Some(descriptor) = manifest_ret {
                 record_native_abi_return(ctx, descriptor, &lowered, name);
@@ -1812,6 +1816,16 @@ pub fn try_lower_extern_func_call(
             // return value directly (no sitofp needed).
             ctx.pending_declares.push((name.clone(), DOUBLE, arg_types));
             let raw = ctx.block().call(DOUBLE, name, &arg_slices);
+            // #10779: this arm serves BOTH a C `double` return (raw native
+            // bits) and Perry's own double ABI (already a NaN box, whose tags
+            // canonicalising would destroy). Gate strictly on the manifest
+            // saying `F64`; `JsValue` or no descriptor is left untouched.
+            let is_native_f64 = matches!(manifest_ret, Some(NativeAbiType::F64));
+            let raw = if is_native_f64 {
+                crate::expr::nanbox_inline::canonicalize_lane_f64(ctx.block(), &raw)
+            } else {
+                raw
+            };
             if let Some(descriptor) = manifest_ret {
                 let lowered = if matches!(descriptor, NativeAbiType::JsValue) {
                     LoweredValue::js_value(raw.clone())