perf(codegen): a truthiness test asks a loop region for no Number lane

changelog.d/11856-regfix-region-truthiness.md

@@ -0,0 +1,5 @@
+A value only tested for truthiness (a conditional's test, a `!` operand) no
+longer asks a loop region for a Number lane, directly or through an
+accumulator's value flow. A string field tested that way failed the region's
+value test on every iteration: #10495 `own3` 246 -> 228 instructions per
+operation.

crates/perry-codegen/src/collectors/ptr_shape_numeric.rs

@@ -598,6 +598,22 @@ pub(crate) fn region_number_flow_reads(
                 locals.push(*id);
                 return;
             }
+            // A value only tested for truthiness does not flow into the
+            // result: a conditional's test (its arms are the values) and a
+            // `!` operand (the result is a Boolean).
+            Expr::Conditional {
+                then_expr,
+                else_expr,
+                ..
+            } => {
+                deps(then_expr, reads, locals);
+                deps(else_expr, reads, locals);
+                return;
+            }
+            Expr::Unary {
+                op: perry_hir::UnaryOp::Not,
+                ..
+            } => return,
             _ => {}
         }
         perry_hir::walker::walk_expr_children(e, &mut |child| deps(child, reads, locals));

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

@@ -17,7 +17,7 @@ use super::class_field_barrier_tests::ir_opts;
 use crate::{compile_module, CompileOptions};
 use perry_hir::types::Type;
 use perry_hir::{
-    BinaryOp, CompareOp, Expr, Function, Module, ModuleInitKind, Param, Stmt, UpdateOp,
+    BinaryOp, CompareOp, Expr, Function, Module, ModuleInitKind, Param, Stmt, UnaryOp, UpdateOp,
 };
 use std::collections::{HashMap, HashSet};
 
@@ -562,6 +562,72 @@ fn a_receiver_read_beneath_a_property_access_requests_no_number_lane() {
     assert!(!ir.contains("rloop.guard.value"), "no R, so no value test");
 }
 
+/// A value only tested for truthiness is not a Number operand: neither a
+/// conditional's test (`h += o.x ? 1 : 0`; the arms are the values) nor a `!`
+/// operand asks the region for R, directly or through the accumulator's
+/// value flow. A string `x` would otherwise fail the value test on every
+/// iteration and the loop would pay the guard for nothing.
+#[test]
+fn a_truthiness_test_requests_no_number_lane() {
+    let ternary = |test: Expr| Expr::Conditional {
+        condition: Box::new(test),
+        then_expr: Box::new(Expr::Integer(1)),
+        else_expr: Box::new(Expr::Integer(0)),
+    };
+    for (name, test) in [
+        ("region_loop_truthy_cond", get("x")),
+        (
+            "region_loop_truthy_not",
+            Expr::Unary {
+                op: UnaryOp::Not,
+                operand: Box::new(get("x")),
+            },
+        ),
+    ] {
+        let ir = loop_ir(
+            name,
+            vec![Stmt::Expr(Expr::LocalSet(
+                H,
+                Box::new(Expr::Binary {
+                    op: BinaryOp::Add,
+                    left: Box::new(Expr::LocalGet(H)),
+                    right: Box::new(ternary(test)),
+                }),
+            ))],
+        );
+        let masks = prime_rep_masks(&ir);
+        assert!(
+            masks.iter().all(|&m| m == 0),
+            "{name}: a truthiness test is not a Number operand: {masks:?}\n{ir}"
+        );
+        assert!(
+            !ir.contains("rloop.guard.value"),
+            "{name}: no R, so no value test"
+        );
+    }
+    // Control: the same read as an arm IS the value, and asks for R.
+    let ir = loop_ir(
+        "region_loop_truthy_arm",
+        vec![Stmt::Expr(Expr::LocalSet(
+            H,
+            Box::new(Expr::Binary {
+                op: BinaryOp::Add,
+                left: Box::new(Expr::LocalGet(H)),
+                right: Box::new(Expr::Conditional {
+                    condition: Box::new(Expr::LocalGet(N)),
+                    then_expr: Box::new(get("x")),
+                    else_expr: Box::new(Expr::Integer(0)),
+                }),
+            }),
+        ))],
+    );
+    let masks = prime_rep_masks(&ir);
+    assert!(
+        !masks.is_empty() && masks.iter().all(|&m| m == 1),
+        "an arm is a Number operand: {masks:?}\n{ir}"
+    );
+}
+
 /// The F-local fixed point follows the fresh F64 read through a temporary and
 /// a loop-carried accumulator. Entry is strict; G and post-loop code retain
 /// the ordinary dynamic add. Removing the scoped materialization or the

crates/perry-codegen/src/stmt/region_loop/plan.rs

@@ -95,7 +95,9 @@ pub(super) fn fact_tree_leaves<'e>(
 /// another property access is that access's RECEIVER (`o.a.length` reads
 /// `o.a` as an object), and a read beneath an element access or a call is
 /// that operation's key, receiver or argument: none of them is a Number
-/// operand, so none may ask the region for a Number lane (R).
+/// operand, so none may ask the region for a Number lane (R). Nor is a value
+/// only tested for truthiness: a conditional's test (`o.a ? 1 : 0`; its arms
+/// are the values) and a `!` operand.
 fn number_operand_reads(e: &Expr, out: &mut Vec<(usize, Recv, String)>) {
     match e {
         Expr::PropertyGet {
@@ -109,6 +111,18 @@ fn number_operand_reads(e: &Expr, out: &mut Vec<(usize, Recv, String)>) {
         Expr::IndexGet { .. } | Expr::Call { .. } | Expr::CallSpread { .. } | Expr::New { .. } => {
             return
         }
+        Expr::Conditional {
+            then_expr,
+            else_expr,
+            ..
+        } => {
+            number_operand_reads(then_expr, out);
+            number_operand_reads(else_expr, out);
+            return;
+        }
+        Expr::Unary {
+            op: UnaryOp::Not, ..
+        } => return,
         _ => {}
     }
     perry_hir::walker::walk_expr_children(e, &mut |child| number_operand_reads(child, out));