cranelift: Fix iadd_carry/iadd_cout in the interpreter (#5176)

bytecodealliance · Nov 14, 2022 · ff46bba · ff46bba
1 parent d3692c2
commit ff46bba
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 4 deletions.
diff --git a/cranelift/filetests/filetests/runtests/iaddcarry.clif b/cranelift/filetests/filetests/runtests/iaddcarry.clif
@@ -11,6 +11,7 @@ block0(v0: i8, v1: i8, v2: i8):
 ; run: %iaddcarry_i8_v(100, 27, 0) == 127
 ; run: %iaddcarry_i8_v(127, 127, 1) == -1
 ; run: %iaddcarry_i8_v(127, 127, 0) == -2
+; run: %iaddcarry_i8_v(-128, -128, 0) == 0
 
 function %iaddcarry_i8_c(i8, i8, i8) -> i8 {
 block0(v0: i8, v1: i8, v2: i8):
@@ -23,6 +24,7 @@ block0(v0: i8, v1: i8, v2: i8):
 ; run: %iaddcarry_i8_c(100, 27, 0) == 0
 ; run: %iaddcarry_i8_c(127, 127, 1) == 1
 ; run: %iaddcarry_i8_c(127, 127, 0) == 1
+; run: %iaddcarry_i8_c(-128, -128, 0) == 1
 
 function %iaddcarry_i16_v(i16, i16, i8) -> i16 {
 block0(v0: i16, v1: i16, v2: i8):

diff --git a/cranelift/filetests/filetests/runtests/iaddcout.clif b/cranelift/filetests/filetests/runtests/iaddcout.clif
@@ -9,6 +9,7 @@ block0(v0: i8, v1: i8):
 ; run: %iaddcout_i8_v(100, 27) == 127
 ; run: %iaddcout_i8_v(100, -20) == 80
 ; run: %iaddcout_i8_v(100, 28) == -128
+; run: %iaddcout_i8_v(-128, -128) == 0
 
 function %iaddcout_i8_c(i8, i8) -> i8 {
 block0(v0: i8, v1: i8):
@@ -19,6 +20,7 @@ block0(v0: i8, v1: i8):
 ; run: %iaddcout_i8_c(100, 27) == 0
 ; run: %iaddcout_i8_c(100, -20) == 0
 ; run: %iaddcout_i8_c(100, 28) == 1
+; run: %iaddcout_i8_c(-128, -128) == 1
 
 function %iaddcout_i16_v(i16, i16) -> i16 {
 block0(v0: i16, v1: i16):

diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs
@@ -743,17 +743,20 @@ where
         ),
         Opcode::IaddIfcin => unimplemented!("IaddIfcin"),
         Opcode::IaddCout => {
-            let sum = Value::add(arg(0)?, arg(1)?)?;
-            let carry = Value::lt(&sum, &arg(0)?)? && Value::lt(&sum, &arg(1)?)?;
+            let carry = arg(0)?.checked_add(arg(1)?)?.is_none();
+            let sum = arg(0)?.add(arg(1)?)?;
             assign_multiple(&[sum, Value::bool(carry, false, types::I8)?])
         }
         Opcode::IaddIfcout => unimplemented!("IaddIfcout"),
         Opcode::IaddCarry => {
             let mut sum = Value::add(arg(0)?, arg(1)?)?;
+            let mut carry = arg(0)?.checked_add(arg(1)?)?.is_none();
+
             if Value::into_bool(arg(2)?)? {
-                sum = Value::add(sum, Value::int(1, ctrl_ty)?)?
+                carry |= sum.clone().checked_add(Value::int(1, ctrl_ty)?)?.is_none();
+                sum = Value::add(sum, Value::int(1, ctrl_ty)?)?;
             }
-            let carry = Value::lt(&sum, &arg(0)?)? && Value::lt(&sum, &arg(1)?)?;
+
             assign_multiple(&[sum, Value::bool(carry, false, types::I8)?])
         }
         Opcode::IaddIfcarry => unimplemented!("IaddIfcarry"),
@@ -1153,6 +1156,7 @@ where
                         let x = u128::min(x as u128, max as u128);
                         x as i128
                     };
+
                     V::int(x, ctrl_ty.lane_type())
                 }
             };

diff --git a/cranelift/interpreter/src/value.rs b/cranelift/interpreter/src/value.rs
@@ -56,6 +56,7 @@ pub trait Value: Clone + From<DataValue> {
     fn sqrt(self) -> ValueResult<Self>;
     fn fma(self, a: Self, b: Self) -> ValueResult<Self>;
     fn abs(self) -> ValueResult<Self>;
+    fn checked_add(self, other: Self) -> ValueResult<Option<Self>>;
 
     // Float operations
     fn neg(self) -> ValueResult<Self>;
@@ -187,6 +188,12 @@ macro_rules! binary_match {
             _ => unimplemented!()
         }
     };
+    ( option $op:ident($arg1:expr, $arg2:expr); [ $( $data_value_ty:ident ),* ] ) => {
+        match ($arg1, $arg2) {
+            $( (DataValue::$data_value_ty(a), DataValue::$data_value_ty(b)) => { Ok(a.$op(*b).map(DataValue::$data_value_ty)) } )*
+            _ => unimplemented!()
+        }
+    };
     ( $op:tt($arg1:expr, $arg2:expr); [ $( $data_value_ty:ident ),* ] ) => {
         match ($arg1, $arg2) {
             $( (DataValue::$data_value_ty(a), DataValue::$data_value_ty(b)) => { Ok(DataValue::$data_value_ty(a $op b)) } )*
@@ -615,6 +622,10 @@ impl Value for DataValue {
         unary_match!(abs(&self); [F32, F64])
     }
 
+    fn checked_add(self, other: Self) -> ValueResult<Option<Self>> {
+        binary_match!(option checked_add(&self, &other); [I8, I16, I32, I64, I128, U8, U16, U32, U64, U128])
+    }
+
     fn neg(self) -> ValueResult<Self> {
         unary_match!(neg(&self); [F32, F64])
     }