Repository: bytecodealliance/wasmtime
Pull request #6018 (2023-03-14 17:36:18)

cranelift: Exclude the control type in narrower and wider
In #6013 we added support for two new constraints in the instruction DSL: `narrower` and `wider`. While these were meant to replace the special-case typechecking in the verifier, they didn't replicate the logic there exactly: both constraints would include the original control type in the sets they produced.

This PR corrects this bug by excluding the control type in the type sets produced when constraining the control type with `narrower` and `wider`.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

- [ ] This has been discussed in issue #..., or if not, please tell us why
  here.
- [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.
- [ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the [code of
conduct](https://github.com/bytecodealliance/wasmtime/blob/master/CODE_OF_CONDUCT.md).
-->



Patch:
diff --git a/cranelift/codegen/src/ir/instructions.rs b/cranelift/codegen/src/ir/instructions.rs
index a8facc1f0..36e343db5 100644
--- a/cranelift/codegen/src/ir/instructions.rs
+++ b/cranelift/codegen/src/ir/instructions.rs
@@ -777,13 +777,13 @@ impl OperandConstraint {
                 tys.lanes = BitSet::from_range(0, 1);
 
                 if ctrl_type.is_int() {
-                    // The upper bound in from_range is exclusive, so add one here get the closed
-                    // interval of [I8, ctrl_type].
-                    tys.ints = BitSet::from_range(3, ctrl_type_bits as u8 + 1);
+                    // The upper bound in from_range is exclusive, and we want to exclude the
+                    // control type to construct the interval of [I8, ctrl_type).
+                    tys.ints = BitSet::from_range(3, ctrl_type_bits as u8);
                 } else if ctrl_type.is_float() {
-                    // The upper bound in from_range is exclusive, so add one here get the closed
-                    // interval of [F32, ctrl_type].
-                    tys.floats = BitSet::from_range(5, ctrl_type_bits as u8 + 1);
+                    // The upper bound in from_range is exclusive, and we want to exclude the
+                    // control type to construct the interval of [F32, ctrl_type).
+                    tys.floats = BitSet::from_range(5, ctrl_type_bits as u8);
                 } else {
                     panic!("The Narrower constraint only operates on floats or ints");
                 }
@@ -797,13 +797,15 @@ impl OperandConstraint {
                 tys.lanes = BitSet::from_range(0, 1);
 
                 if ctrl_type.is_int() {
-                    // The upper bound should include all types wider than `ctrl_type`, so we use
-                    // `2^8` as the upper bound to define the closed range `[ctrl_type, I128]`.
-                    tys.ints = BitSet::from_range(ctrl_type_bits as u8, 8);
+                    // The interval should include all types wider than `ctrl_type`, so we use
+                    // `2^8` as the upper bound, and add one to the bits of `ctrl_type` to define
+                    // the interval `(ctrl_type, I128]`.
+                    tys.ints = BitSet::from_range(ctrl_type_bits as u8 + 1, 8);
                 } else if ctrl_type.is_float() {
-                    // The upper bound should include all float types wider than `ctrl_type`, so we
-                    // use `2^7` as the upper bound to define the closed range `[ctrl_type, F64]`.
-                    tys.floats = BitSet::from_range(ctrl_type_bits as u8, 7);
+                    // The interval should include all float types wider than `ctrl_type`, so we
+                    // use `2^7` as the upper bound, and add one to the bits of `ctrl_type` to
+                    // define the interval `(ctrl_type, F64]`.
+                    tys.floats = BitSet::from_range(ctrl_type_bits as u8 + 1, 7);
                 } else {
                     panic!("The Wider constraint only operates on floats or ints");
                 }


Test patch:
diff --git a/cranelift/filetests/filetests/verifier/type_check.clif b/cranelift/filetests/filetests/verifier/type_check.clif
index 68de2338e..41ded268c 100644
--- a/cranelift/filetests/filetests/verifier/type_check.clif
+++ b/cranelift/filetests/filetests/verifier/type_check.clif
@@ -114,13 +114,15 @@ function %bad_extend() {
 block0:
     v0 = iconst.i32 10
     v1 = uextend.i16 v0 ; error: arg 0 (v0) with type i32 failed to satisfy type set
+    v2 = uextend.i32 v0 ; error: arg 0 (v0) with type i32 failed to satisfy type set
     return
 }
 
 function %bad_reduce() {
 block0:
     v0 = iconst.i32 10
-    v1 = ireduce.i64 v0 ; error: arg 0 (v0) with type i32 failed to satisfy type set
+    v1 = ireduce.i32 v0 ; error: arg 0 (v0) with type i32 failed to satisfy type set
+    v2 = ireduce.i64 v0 ; error: arg 0 (v0) with type i32 failed to satisfy type set
     return
 }
 
@@ -128,6 +130,7 @@ function %bad_fdemote() {
 block0:
     v0 = f32const 0xf.f
     v1 = fdemote.f64 v0 ; error: arg 0 (v0) with type f32 failed to satisfy type set
+    v2 = fdemote.f32 v0 ; error: arg 0 (v0) with type f32 failed to satisfy type set
     return
 }
 
@@ -135,6 +138,7 @@ function %bad_fpromote() {
 block0:
     v0 = f64const 0xf.f
     v1 = fpromote.f32 v0 ; error: arg 0 (v0) with type f64 failed to satisfy type set
+    v2 = fpromote.f64 v0 ; error: arg 0 (v0) with type f64 failed to satisfy type set
     return
 }