YJIT: Use enum for expressing type diff (#7370)

Merged-By: maximecb <maximecb@ruby-lang.org>

yjit/src/codegen.rs

@@ -292,7 +292,7 @@ fn verify_ctx(jit: &JITState, ctx: &Context) {
     let self_val_type = Type::from(self_val);
 
     // Verify self operand type
-    if self_val_type.diff(ctx.get_opnd_type(SelfOpnd)) == usize::MAX {
+    if self_val_type.diff(ctx.get_opnd_type(SelfOpnd)) == TypeDiff::Incompatible {
         panic!(
             "verify_ctx: ctx self type ({:?}) incompatible with actual value of self {}",
             ctx.get_opnd_type(SelfOpnd),
@@ -333,7 +333,7 @@ fn verify_ctx(jit: &JITState, ctx: &Context) {
         }
 
         // If the actual type differs from the learned type
-        if val_type.diff(learned_type) == usize::MAX {
+        if val_type.diff(learned_type) == TypeDiff::Incompatible {
             panic!(
                 "verify_ctx: ctx type ({:?}) incompatible with actual value on stack: {}",
                 learned_type,
@@ -350,7 +350,7 @@ fn verify_ctx(jit: &JITState, ctx: &Context) {
         let local_val = jit.peek_at_local(i as i32);
         let local_type = Type::from(local_val);
 
-        if local_type.diff(learned_type) == usize::MAX {
+        if local_type.diff(learned_type) == TypeDiff::Incompatible {
             panic!(
                 "verify_ctx: ctx type ({:?}) incompatible with actual value of local: {} (type {:?})",
                 learned_type,
@@ -1314,7 +1314,7 @@ fn guard_object_is_heap(
     asm.cmp(object, Qfalse.into());
     asm.je(side_exit);
 
-    if object_type.diff(Type::UnknownHeap) != usize::MAX {
+    if object_type.diff(Type::UnknownHeap) != TypeDiff::Incompatible {
         ctx.upgrade_opnd_type(object_opnd, Type::UnknownHeap);
     }
 }
@@ -1347,7 +1347,7 @@ fn guard_object_is_array(
     asm.cmp(flags_opnd, (RUBY_T_ARRAY as u64).into());
     asm.jne(side_exit);
 
-    if object_type.diff(Type::TArray) != usize::MAX {
+    if object_type.diff(Type::TArray) != TypeDiff::Incompatible {
         ctx.upgrade_opnd_type(object_opnd, Type::TArray);
     }
 }
@@ -8066,7 +8066,7 @@ mod tests {
         asm.compile(&mut cb);
 
         assert_eq!(status, KeepCompiling);
-        assert_eq!(context.diff(&Context::default()), 0);
+        assert_eq!(context.diff(&Context::default()), TypeDiff::Compatible(0));
         assert_eq!(cb.get_write_pos(), 0);
     }
 
@@ -8078,7 +8078,7 @@ mod tests {
         let status = gen_pop(&mut jit, &mut context, &mut asm, &mut ocb);
 
         assert_eq!(status, KeepCompiling);
-        assert_eq!(context.diff(&Context::default()), 0);
+        assert_eq!(context.diff(&Context::default()), TypeDiff::Compatible(0));
     }
 
     #[test]

yjit/src/core.rs

@@ -221,53 +221,58 @@ impl Type {
     }
 
     /// Compute a difference between two value types
-    /// Returns 0 if the two are the same
+    pub fn diff(self, dst: Self) -> TypeDiff {
-    /// Returns > 0 if different but compatible
-    /// Returns usize::MAX if incompatible
-    pub fn diff(self, dst: Self) -> usize {
         // Perfect match, difference is zero
         if self == dst {
-            return 0;
+            return TypeDiff::Compatible(0);
         }
 
         // Any type can flow into an unknown type
         if dst == Type::Unknown {
-            return 1;
+            return TypeDiff::Compatible(1);
         }
 
         // A CString is also a TString.
         if self == Type::CString && dst == Type::TString {
-            return 1;
+            return TypeDiff::Compatible(1);
         }
 
         // A CArray is also a TArray.
         if self == Type::CArray && dst == Type::TArray {
-            return 1;
+            return TypeDiff::Compatible(1);
         }
 
         // Specific heap type into unknown heap type is imperfect but valid
         if self.is_heap() && dst == Type::UnknownHeap {
-            return 1;
+            return TypeDiff::Compatible(1);
         }
 
         // Specific immediate type into unknown immediate type is imperfect but valid
         if self.is_imm() && dst == Type::UnknownImm {
-            return 1;
+            return TypeDiff::Compatible(1);
         }
 
         // Incompatible types
-        return usize::MAX;
+        return TypeDiff::Incompatible;
     }
 
     /// Upgrade this type into a more specific compatible type
     /// The new type must be compatible and at least as specific as the previously known type.
     fn upgrade(&mut self, src: Self) {
         // Here we're checking that src is more specific than self
-        assert!(src.diff(*self) != usize::MAX);
+        assert!(src.diff(*self) != TypeDiff::Incompatible);
         *self = src;
     }
 }
 
+#[derive(Debug, Eq, PartialEq)]
+pub enum TypeDiff {
+    // usize == 0: Same type
+    // usize >= 1: Different but compatible. The smaller, the more compatible.
+    Compatible(usize),
+    Incompatible,
+}
+
 // Potential mapping of a value on the temporary stack to
 // self, a local variable or constant so that we can track its type
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
@@ -967,14 +972,15 @@ fn find_block_version(blockid: BlockId, ctx: &Context) -> Option<BlockRef> {
     // For each version matching the blockid
     for blockref in versions.iter_mut() {
         let block = blockref.borrow();
-        let diff = ctx.diff(&block.ctx);
-
         // Note that we always prefer the first matching
         // version found because of inline-cache chains
-        if diff < best_diff {
+        match ctx.diff(&block.ctx) {
+            TypeDiff::Compatible(diff) if diff < best_diff => {
                 best_version = Some(blockref.clone());
                 best_diff = diff;
             }
+            _ => {}
+        }
     }
 
     // If greedy versioning is enabled
@@ -1005,7 +1011,7 @@ pub fn limit_block_versions(blockid: BlockId, ctx: &Context) -> Context {
         generic_ctx.sp_offset = ctx.sp_offset;
 
         debug_assert_ne!(
-            usize::MAX,
+            TypeDiff::Incompatible,
             ctx.diff(&generic_ctx),
             "should substitute a compatible context",
         );
@@ -1468,55 +1474,46 @@ impl Context {
     }
 
     /// Compute a difference score for two context objects
-    /// Returns 0 if the two contexts are the same
+    pub fn diff(&self, dst: &Context) -> TypeDiff {
-    /// Returns > 0 if different but compatible
-    /// Returns usize::MAX if incompatible
-    pub fn diff(&self, dst: &Context) -> usize {
         // Self is the source context (at the end of the predecessor)
         let src = self;
 
         // Can only lookup the first version in the chain
         if dst.chain_depth != 0 {
-            return usize::MAX;
+            return TypeDiff::Incompatible;
         }
 
         // Blocks with depth > 0 always produce new versions
         // Sidechains cannot overlap
         if src.chain_depth != 0 {
-            return usize::MAX;
+            return TypeDiff::Incompatible;
         }
 
         if dst.stack_size != src.stack_size {
-            return usize::MAX;
+            return TypeDiff::Incompatible;
         }
 
         if dst.sp_offset != src.sp_offset {
-            return usize::MAX;
+            return TypeDiff::Incompatible;
         }
 
         // Difference sum
         let mut diff = 0;
 
         // Check the type of self
-        let self_diff = src.self_type.diff(dst.self_type);
+        diff += match src.self_type.diff(dst.self_type) {
-
+            TypeDiff::Compatible(diff) => diff,
-        if self_diff == usize::MAX {
+            TypeDiff::Incompatible => return TypeDiff::Incompatible,
-            return usize::MAX;
+        };
-        }
-
-        diff += self_diff;
 
         // For each local type we track
         for i in 0..src.local_types.len() {
             let t_src = src.local_types[i];
             let t_dst = dst.local_types[i];
-            let temp_diff = t_src.diff(t_dst);
+            diff += match t_src.diff(t_dst) {
-
+                TypeDiff::Compatible(diff) => diff,
-            if temp_diff == usize::MAX {
+                TypeDiff::Incompatible => return TypeDiff::Incompatible,
-                return usize::MAX;
+            };
-            }
-
-            diff += temp_diff;
         }
 
         // For each value on the temp stack
@@ -1531,20 +1528,17 @@ impl Context {
                     // stack operand.
                     diff += 1;
                 } else {
-                    return usize::MAX;
+                    return TypeDiff::Incompatible;
                 }
             }
 
-            let temp_diff = src_type.diff(dst_type);
+            diff += match src_type.diff(dst_type) {
-
+                TypeDiff::Compatible(diff) => diff,
-            if temp_diff == usize::MAX {
+                TypeDiff::Incompatible => return TypeDiff::Incompatible,
-                return usize::MAX;
+            };
         }
 
-            diff += temp_diff;
+        return TypeDiff::Compatible(diff);
-        }
-
-        return diff;
     }
 
     pub fn two_fixnums_on_stack(&self, jit: &mut JITState) -> Option<bool> {
@@ -2509,22 +2503,22 @@ mod tests {
     #[test]
     fn types() {
         // Valid src => dst
-        assert_eq!(Type::Unknown.diff(Type::Unknown), 0);
+        assert_eq!(Type::Unknown.diff(Type::Unknown), TypeDiff::Compatible(0));
-        assert_eq!(Type::UnknownImm.diff(Type::UnknownImm), 0);
+        assert_eq!(Type::UnknownImm.diff(Type::UnknownImm), TypeDiff::Compatible(0));
-        assert_ne!(Type::UnknownImm.diff(Type::Unknown), usize::MAX);
+        assert_ne!(Type::UnknownImm.diff(Type::Unknown), TypeDiff::Incompatible);
-        assert_ne!(Type::Fixnum.diff(Type::Unknown), usize::MAX);
+        assert_ne!(Type::Fixnum.diff(Type::Unknown), TypeDiff::Incompatible);
-        assert_ne!(Type::Fixnum.diff(Type::UnknownImm), usize::MAX);
+        assert_ne!(Type::Fixnum.diff(Type::UnknownImm), TypeDiff::Incompatible);
 
         // Invalid src => dst
-        assert_eq!(Type::Unknown.diff(Type::UnknownImm), usize::MAX);
+        assert_eq!(Type::Unknown.diff(Type::UnknownImm), TypeDiff::Incompatible);
-        assert_eq!(Type::Unknown.diff(Type::Fixnum), usize::MAX);
+        assert_eq!(Type::Unknown.diff(Type::Fixnum), TypeDiff::Incompatible);
-        assert_eq!(Type::Fixnum.diff(Type::UnknownHeap), usize::MAX);
+        assert_eq!(Type::Fixnum.diff(Type::UnknownHeap), TypeDiff::Incompatible);
     }
 
     #[test]
     fn context() {
         // Valid src => dst
-        assert_eq!(Context::default().diff(&Context::default()), 0);
+        assert_eq!(Context::default().diff(&Context::default()), TypeDiff::Compatible(0));
 
         // Try pushing an operand and getting its type
         let mut ctx = Context::default();