ZJIT: ivar code cleanup

I just wanted to clean up some of the IV handling code in ZJIT.
If classes, modules, or TDATA have instance variables, and they aren't
"too complex", the instance variables will always be in an external
buffer. Since we can make these assertions at compile time, it allows us
to clean up some of this code

Author: tenderlove

jit.c

@@ -560,20 +560,6 @@ rb_jit_multi_ractor_p(void)
     return rb_multi_ractor_p();
 }
 
-bool
-rb_jit_class_fields_embedded_p(VALUE klass)
-{
-    VALUE fields_obj = RCLASS_EXT_PRIME(klass)->fields_obj;
-    return !fields_obj || !FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP);
-}
-
-bool
-rb_jit_data_fields_embedded_p(VALUE obj)
-{
-    VALUE fields_obj = RTYPEDDATA(obj)->fields_obj;
-    return !fields_obj || !FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP);
-}
-
 // Acquire the VM lock and then signal all other Ruby threads (ractors) to
 // contend for the VM lock, putting them to sleep. ZJIT and YJIT use this to
 // evict threads running inside generated code so among other things, it can

zjit/bindgen/src/main.rs

@@ -319,8 +319,6 @@ fn main() {
         .allowlist_function("rb_assert_holding_vm_lock")
         .allowlist_function("rb_jit_shape_complex_p")
         .allowlist_function("rb_jit_multi_ractor_p")
-        .allowlist_function("rb_jit_class_fields_embedded_p")
-        .allowlist_function("rb_jit_data_fields_embedded_p")
         .allowlist_function("rb_jit_vm_lock_then_barrier")
         .allowlist_function("rb_jit_vm_unlock")
         .allowlist_function("rb_jit_for_each_iseq")

zjit/src/cruby.rs

@@ -629,19 +629,11 @@ impl VALUE {
         }
     }
 
-    pub fn class_fields_embedded_p(self) -> bool {
-        unsafe { rb_jit_class_fields_embedded_p(self) }
-    }
-
     pub fn data_p(self) -> bool {
         !self.special_const_p() &&
             self.builtin_type() == RUBY_T_DATA
     }
 
-    pub fn data_fields_embedded_p(self) -> bool {
-        unsafe { rb_jit_data_fields_embedded_p(self) }
-    }
-
     pub fn as_fixnum(self) -> i64 {
         assert!(self.fixnum_p());
         (self.0 as i64) >> 1

zjit/src/cruby_bindings.inc.rs

@@ -4373,31 +4373,6 @@ impl Function {
         })
     }
 
-    fn load_ivar_from_fields(&mut self, block: BlockId, recv: InsnId, is_embedded: bool, id: ID, ivar_index: attr_index_t) -> InsnId {
-        if is_embedded {
-            return self.load_ivar_embedded(block, recv, id, ivar_index);
-        } else {
-            return self.load_ivar_heap(block, recv, id, ivar_index);
-        }
-    }
-
-    /// This puts a guard that establishes the preconditon for [Self::load_ivar]
-    fn load_ivar_guard_type(&mut self, block: BlockId, recv: InsnId, recv_type: ProfiledType, state: InsnId) -> InsnId {
-        if recv_type.flags().is_t_class() {
-            // Check class first since `Class < Module`
-            self.push_insn(block, Insn::GuardType { val: recv, guard_type: types::Class, state, recompile: None })
-        } else if recv_type.flags().is_t_module() {
-            self.push_insn(block, Insn::GuardType { val: recv, guard_type: types::Module, state, recompile: None })
-        } else if recv_type.flags().is_t_data() {
-            self.push_insn(block, Insn::GuardType { val: recv, guard_type: types::TData, state, recompile: None })
-        } else {
-            // HeapBasicObject is wider than T_OBJECT, but shapes for T_OBJECTs are in a pool of
-            // its own and are guaranteed to be different from shapes of any other T_* types. So
-            // the shape check that follows already covers checking for T_OBJECT.
-            self.push_insn(block, Insn::GuardType { val: recv, guard_type: types::HeapBasicObject, state, recompile: None })
-        }
-    }
-
     fn load_ivar(&mut self, block: BlockId, self_val: InsnId, recv_type: ProfiledType, id: ID) -> InsnId {
         // Too-complex shapes use hash tables; rb_shape_get_iv_index doesn't support them.
         // Callers must filter these out before calling load_ivar.
@@ -4425,10 +4400,14 @@ impl Function {
                     offset,
                     return_type: types::RubyValue,
                 });
-                self.load_ivar_from_fields(block, fields_obj, recv_type.flags().is_fields_embedded(), id, ivar_index)
+                self.load_ivar_embedded(block, fields_obj, id, ivar_index)
             },
             ShapeLayout::RObject => {
-                self.load_ivar_from_fields(block, self_val, recv_type.flags().is_embedded(), id, ivar_index)
+                if recv_type.flags().is_embedded() {
+                    self.load_ivar_embedded(block, self_val, id, ivar_index)
+                } else {
+                    self.load_ivar_heap(block, self_val, id, ivar_index)
+                }
             },
             ShapeLayout::Other => {
                 // Non-T_OBJECT, non-class/module, non-typed-data: fall back to C call
@@ -4470,7 +4449,7 @@ impl Function {
                             // need to wrap it again here.
                             self.push_insn_id(block, insn_id); continue;
                         }
-                        let self_val = self.load_ivar_guard_type(block, self_val, recv_type, state);
+                        let self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: types::HeapBasicObject, state, recompile: None });
                         let shape = self.load_shape(block, self_val);
                         self.guard_shape(block, shape, recv_type.shape(), state, Some(Recompile::ProfileSelf));
                         let replacement = self.load_ivar(block, self_val, recv_type, id);
@@ -4503,7 +4482,7 @@ impl Function {
                             // On the final version, keep the DefinedIvar fallback instead of another shape guard.
                             self.push_insn_id(block, insn_id); continue;
                         }
-                        let self_val = self.load_ivar_guard_type(block, self_val, recv_type, state);
+                        let self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: types::HeapBasicObject, state, recompile: None });
                         let shape = self.load_shape(block, self_val);
                         self.guard_shape(block, shape, recv_type.shape(), state, Some(Recompile::ProfileSelf));
                         let mut ivar_index: attr_index_t = 0;
@@ -4581,7 +4560,7 @@ impl Function {
                             }
                             // Fall through to emitting the ivar write
                         }
-                        let self_val = self.load_ivar_guard_type(block, self_val, recv_type, state);
+                        let self_val = self.push_insn(block, Insn::GuardType { val: self_val, guard_type: types::HeapBasicObject, state, recompile: None });
                         let shape = self.load_shape(block, self_val);
                         // TODO: attr_writer SetIvar has a null inline cache and may target a receiver
                         // operand other than CFP self. Support it with a reprofile strategy that

zjit/src/hir.rs

@@ -213,14 +213,6 @@ impl Flags {
     const IS_STRUCT_EMBEDDED: u32 = 1 << 3;
     /// Set if the ProfiledType is used for profiling specific objects, not just classes/shapes
     const IS_OBJECT_PROFILING: u32 = 1 << 4;
-    /// Class/module fields_obj is embedded (or absent)
-    const IS_FIELDS_EMBEDDED: u32 = 1 << 5;
-    /// Object is a T_CLASS
-    const IS_T_CLASS: u32 = 1 << 6;
-    /// Object is a T_MODULE
-    const IS_T_MODULE: u32 = 1 << 7;
-    /// Object is a T_DATA
-    const IS_T_DATA: u32 = 1 << 8;
 
     pub fn none() -> Self { Self(Self::NONE) }
 
@@ -230,10 +222,6 @@ impl Flags {
     pub fn is_t_object(self) -> bool { (self.0 & Self::IS_T_OBJECT) != 0 }
     pub fn is_struct_embedded(self) -> bool { (self.0 & Self::IS_STRUCT_EMBEDDED) != 0 }
     pub fn is_object_profiling(self) -> bool { (self.0 & Self::IS_OBJECT_PROFILING) != 0 }
-    pub fn is_fields_embedded(self) -> bool { (self.0 & Self::IS_FIELDS_EMBEDDED) != 0 }
-    pub fn is_t_class(self) -> bool { (self.0 & Self::IS_T_CLASS) != 0 }
-    pub fn is_t_module(self) -> bool { (self.0 & Self::IS_T_MODULE) != 0 }
-    pub fn is_t_data(self) -> bool { (self.0 & Self::IS_T_DATA) != 0 }
 }
 
 /// opt_send_without_block/opt_plus/... should store:
@@ -310,24 +298,6 @@ impl ProfiledType {
         if unsafe { RB_TYPE_P(obj, RUBY_T_OBJECT) } {
             flags.0 |= Flags::IS_T_OBJECT;
         }
-        if unsafe { RB_TYPE_P(obj, RUBY_T_CLASS) } {
-            flags.0 |= Flags::IS_T_CLASS;
-            if obj.class_fields_embedded_p() {
-                flags.0 |= Flags::IS_FIELDS_EMBEDDED;
-            }
-        }
-        if unsafe { RB_TYPE_P(obj, RUBY_T_MODULE) } {
-            flags.0 |= Flags::IS_T_MODULE;
-            if obj.class_fields_embedded_p() {
-                flags.0 |= Flags::IS_FIELDS_EMBEDDED;
-            }
-        }
-        if obj.data_p() {
-            flags.0 |= Flags::IS_T_DATA;
-            if obj.data_fields_embedded_p() {
-                flags.0 |= Flags::IS_FIELDS_EMBEDDED;
-            }
-        }
         Self { class: obj.class_of(), shape: obj.shape_id_of(), flags }
     }