interpreter: lower && and || as short-circuit branches

interpreter/src/ir.rs

@@ -48,8 +48,6 @@ pub enum Instruction {
     Lt { dest: Reg, lhs: Arg, rhs: Arg, tyr: ArgTy, tyl: ArgTy },
     LtE { dest: Reg, lhs: Arg, rhs: Arg, tyr: ArgTy, tyl: ArgTy },
     GtE { dest: Reg, lhs: Arg, rhs: Arg, tyr: ArgTy, tyl: ArgTy },
-    And { dest: Reg, lhs: Arg, rhs: Arg, tyr: ArgTy, tyl: ArgTy },
-    Or { dest: Reg, lhs: Arg, rhs: Arg, tyr: ArgTy, tyl: ArgTy },
     Matches { dest: Reg, lhs: Arg, rhs: Arg, tyr: ArgTy, tyl: ArgTy },
     MatchesNot { dest: Reg, lhs: Arg, rhs: Arg, tyr: ArgTy, tyl: ArgTy },
     Add { dest: Reg, lhs: Arg, rhs: Arg, tyr: ArgTy, tyl: ArgTy },
@@ -155,8 +153,6 @@ impl Display for Instruction {
             | Self::Lt { dest, lhs, rhs, tyl, tyr }
             | Self::LtE { dest, lhs, rhs, tyl, tyr }
             | Self::GtE { dest, lhs, rhs, tyl, tyr }
-            | Self::And { dest, lhs, rhs, tyl, tyr }
-            | Self::Or { dest, lhs, rhs, tyl, tyr }
             | Self::Matches { dest, lhs, rhs, tyl, tyr }
             | Self::MatchesNot { dest, lhs, rhs, tyl, tyr }
             | Self::Add { dest, lhs, rhs, tyl, tyr }
@@ -230,8 +226,6 @@ impl Instruction {
             Self::Lt { .. } => "lt",
             Self::LtE { .. } => "le",
             Self::GtE { .. } => "ge",
-            Self::And { .. } => "and",
-            Self::Or { .. } => "or",
             Self::Matches { .. } => "mtch",
             Self::MatchesNot { .. } => "nmtch",
             Self::Add { .. } => "add",

interpreter/src/ir/lower.rs

@@ -349,18 +349,22 @@ impl<'a> CodeGen<'a> {
                         .emit(Instruction::from_unary(*op, dest, src.to_arg()));
                     src.free(self);
                 }
-                ExprNode::BinaryOperation(op, lhs, rhs) => {
-                    let lhs = self.lower_expr(lhs);
-                    let rhs = self.lower_expr(rhs);
-                    self.bc.emit(Instruction::from_binary(
-                        *op,
-                        dest,
-                        lhs.to_arg(),
-                        rhs.to_arg(),
-                    ));
-                    lhs.free(self);
-                    rhs.free(self);
-                }
+                ExprNode::BinaryOperation(op, lhs, rhs) => match op {
+                    BinaryOperator::And => self.lower_and_into(lhs, rhs, dest),
+                    BinaryOperator::Or => self.lower_or_into(lhs, rhs, dest),
+                    _ => {
+                        let lhs = self.lower_expr(lhs);
+                        let rhs = self.lower_expr(rhs);
+                        self.bc.emit(Instruction::from_binary(
+                            *op,
+                            dest,
+                            lhs.to_arg(),
+                            rhs.to_arg(),
+                        ));
+                        lhs.free(self);
+                        rhs.free(self);
+                    }
+                },
                 ExprNode::Ternary(condition, true_then, false_then) => {
                     let (if_label, state) = self.emit_branch(condition, |this| {
                         RegsState::new(this)
@@ -518,6 +522,44 @@ impl<'a> CodeGen<'a> {
         }
     }
 
+    fn lower_and_into(&mut self, lhs: &Expr<'_>, rhs: &Expr<'_>, dest: Reg) {
+        let (if_label, _) = self.emit_branch(lhs, |this| {
+            let rhs_reg = this.alloc_reg();
+            this.lower_expr_into(rhs, *rhs_reg);
+            this.truthify(*rhs_reg, dest);
+            this.free_reg(rhs_reg);
+        });
+        self.bc.nth(if_label).push_end_label();
+        self.emit_jump(|this| {
+            this.bc
+                .emit(Instruction::Copy { dest, arg: Arg { imm: 0 }, ty: ArgTy::Imm });
+        });
+    }
+
+    fn lower_or_into(&mut self, lhs: &Expr<'_>, rhs: &Expr<'_>, dest: Reg) {
+        let (if_label, _) = self.emit_branch(lhs, |this| {
+            this.bc
+                .emit(Instruction::Copy { dest, arg: Arg { imm: 1 }, ty: ArgTy::Imm });
+        });
+        self.bc.nth(if_label).push_end_label();
+        self.emit_jump(|this| {
+            let rhs_reg = this.alloc_reg();
+            this.lower_expr_into(rhs, *rhs_reg);
+            this.truthify(*rhs_reg, dest);
+            this.free_reg(rhs_reg);
+        });
+    }
+
+    /// Coerce `src` to an integer truth value (0 or 1), as gawk does via `mkbool()`.
+    fn truthify(&mut self, src: Reg, dest: Reg) {
+        let tmp = self.alloc_reg();
+        self.bc
+            .emit(Instruction::Negation { dest: *tmp, arg: Arg { reg: src }, ty: ArgTy::Reg });
+        self.bc
+            .emit(Instruction::Negation { dest, arg: Arg { reg: *tmp }, ty: ArgTy::Reg });
+        self.free_reg(tmp);
+    }
+
     fn emit_branch<T>(
         &mut self,
         condition_expr: &Expr<'_>,
@@ -723,8 +765,9 @@ impl Instruction {
             BinaryOperator::Lt => Self::Lt { dest, lhs, rhs, tyl, tyr },
             BinaryOperator::LtE => Self::LtE { dest, lhs, rhs, tyl, tyr },
             BinaryOperator::GtE => Self::GtE { dest, lhs, rhs, tyl, tyr },
-            BinaryOperator::And => Self::And { dest, lhs, rhs, tyl, tyr },
-            BinaryOperator::Or => Self::Or { dest, lhs, rhs, tyl, tyr },
+            BinaryOperator::And | BinaryOperator::Or => {
+                unreachable!("&& and || are lowered with branches")
+            }
             BinaryOperator::Matches => Self::Matches { dest, lhs, rhs, tyl, tyr },
             BinaryOperator::MatchesNot => Self::MatchesNot { dest, lhs, rhs, tyl, tyr },
             BinaryOperator::Add => Self::Add { dest, lhs, rhs, tyl, tyr },
@@ -870,6 +913,30 @@ mod tests {
         });
     }
 
+    fn brif_count(cg: &CodeGen<'_>) -> usize {
+        cg.bc
+            .code
+            .iter()
+            .filter(|i| matches!(i, Instruction::Branch { .. }))
+            .count()
+    }
+
+    #[test]
+    fn and_or_use_branches_not_dedicated_ops() {
+        with_lower("BEGIN { print (0 && 1); print (1 || 0) }", |cg| {
+            let bc = format!("{}", cg.bc);
+            assert!(
+                !bc.contains(" <- and "),
+                "unexpected And instruction:\n{bc}"
+            );
+            assert!(!bc.contains(" <- or "), "unexpected Or instruction:\n{bc}");
+            assert!(
+                bc.contains("brif"),
+                "expected short-circuit branches:\n{bc}"
+            );
+        });
+    }
+
     #[test]
     fn switch_default_in_middle_uses_no_match_jump_only() {
         with_lower(
@@ -901,6 +968,15 @@ mod tests {
         });
     }
 
+    #[test]
+    fn chained_and_lowers_one_branch_per_operator() {
+        let mut single = 0;
+        with_lower("BEGIN { print (0 && 1) }", |cg| single = brif_count(cg));
+        with_lower("BEGIN { print (0 && 1 && 2) }", |cg| {
+            assert_eq!(brif_count(cg), single + 1, "each && should add one branch");
+        });
+    }
+
     #[test]
     fn switch_no_exit_jumps_between_case_bodies() {
         with_lower(
@@ -937,4 +1013,13 @@ mod tests {
             );
         });
     }
+
+    #[test]
+    fn chained_or_lowers_one_branch_per_operator() {
+        let mut single = 0;
+        with_lower("BEGIN { print (1 || 0) }", |cg| single = brif_count(cg));
+        with_lower("BEGIN { print (1 || 0 || 2) }", |cg| {
+            assert_eq!(brif_count(cg), single + 1, "each || should add one branch");
+        });
+    }
 }

interpreter/src/vm.rs

@@ -199,8 +199,6 @@ impl Interpreter<'_> {
                 Instruction::GtE { dest, lhs, rhs, tyl, tyr } => {
                     rx!(self, dest, lhs: tyl, rhs: tyr, Value::b2f(lhs >= rhs));
                 }
-                Instruction::And { dest: _, lhs: _, rhs: _, tyr: _, tyl: _ } => todo!(),
-                Instruction::Or { dest: _, lhs: _, rhs: _, tyr: _, tyl: _ } => todo!(),
                 Instruction::Matches { dest, lhs, rhs, tyl, tyr } => {
                     rx!(self, lhs: tyl, rhs: tyr);
                     let matched = match rhs {

tests/by-util/test_awk.rs

@@ -84,6 +84,58 @@ fn switch_no_match_without_default_continues() {
         .stdout_only("done\n");
 }
 
+#[test]
+fn short_circuit_and_or_truth_values() {
+    ucmd()
+        .arg("BEGIN { print (0 && 1); print (1 && 2); print (1 || 0); print (0 || 5) }")
+        .succeeds()
+        .stdout_only("0\n1\n1\n1\n");
+}
+
+#[test]
+fn short_circuit_and_in_if_condition() {
+    ucmd()
+        .arg("BEGIN { if (1 && 0) print 1; else print 0 }")
+        .succeeds()
+        .stdout_only("0\n");
+}
+
+#[test]
+fn short_circuit_or_in_if_condition() {
+    ucmd()
+        .arg("BEGIN { if (1 || 0) print 1; else print 0 }")
+        .succeeds()
+        .stdout_only("1\n");
+}
+
+#[test]
+fn short_circuit_chained_and() {
+    ucmd()
+        .arg("BEGIN { a=1; b=1; c=3; print (a && b && c == 3) }")
+        .succeeds()
+        .stdout_only("1\n");
+    ucmd()
+        .arg("BEGIN { a=1; b=0; c=3; print (a && b && c == 3) }")
+        .succeeds()
+        .stdout_only("0\n");
+}
+
+#[test]
+fn short_circuit_and_skips_rhs_side_effects() {
+    ucmd()
+        .arg("BEGIN { i=0; print (0 && ++i); print i }")
+        .succeeds()
+        .stdout_only("0\n0\n");
+}
+
+#[test]
+fn short_circuit_or_skips_rhs_side_effects() {
+    ucmd()
+        .arg("BEGIN { i=0; print (1 || ++i); print i }")
+        .succeeds()
+        .stdout_only("1\n0\n");
+}
+
 // Regression test for issue #5: writing to /dev/full must not panic.
 #[cfg(target_os = "linux")]
 #[test]