Remove unnecessary guard from YARVINSN_setlocal_wc_0, remove comments, and update tests.

Author: jacob-shops

zjit/src/hir.rs

@@ -6750,11 +6750,11 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
                     }
                 }
                 YARVINSN_setlocal_WC_0 => {
-                    // TODO(Jacob): Modify this to use guards
                     let ep_offset = get_arg(pc, 0).as_u32();
                     const level: u32 = 0;
                     let ep = fun.push_insn(block, Insn::GetEP { level });
                     let val = state.stack_pop()?;
+                    // In ZJIT, Insn::SetLocal only needs to be executed when ep is escaped
                     if ep_escaped || has_blockiseq { // TODO: figure out how to drop has_blockiseq here
                         // Write the local using EP
                         let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state.without_locals() }); // skip spilling locals
@@ -6764,9 +6764,6 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
                     } else if local_inval {
                         // If there has been any non-leaf call since JIT entry or the last patch point,
                         // add a patch point to make sure locals have not been escaped.
-                        let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state.without_locals() }); // skip spilling locals
-                        let flags = fun.push_insn(block, Insn::LoadField { recv: ep, id: ID!(_env_data_index_flags), offset: SIZEOF_VALUE_I32 * (VM_ENV_DATA_INDEX_FLAGS as i32), return_type: types::CInt64 });
-                        fun.push_insn(block, Insn::GuardNoBitsSet { val: flags, mask: Const::CUInt64(VM_ENV_FLAG_WB_REQUIRED.into()), reason: SideExitReason::WriteBarrierRequired, state: exit_id });
                         fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoEPEscape(iseq), state: exit_id });
                         local_inval = false;
                     }
@@ -6778,7 +6775,6 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
                     state.stack_push(fun.push_insn(block, Insn::GetLocal { ep_offset, level: 1, use_sp: false, rest_param: false }));
                 }
                 YARVINSN_setlocal_WC_1 => {
-                    // TODO(Jacob): Modify this to use guards
                     let ep_offset = get_arg(pc, 0).as_u32();
                     const level: u32 = 1;
                     let ep = fun.push_insn(block, Insn::GetEP { level });
@@ -6792,8 +6788,6 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
                     state.stack_push(fun.push_insn(block, Insn::GetLocal { ep_offset, level, use_sp: false, rest_param: false }));
                 }
                 YARVINSN_setlocal => {
-                    // TODO(Jacob): Make sure this works
-                    // TODO(Jacob): Clean up the offset, levels, ep chasing, etc
                     let ep_offset = get_arg(pc, 0).as_u32();
                     let level = get_arg(pc, 1).as_u32();
                     let ep = fun.push_insn(block, Insn::GetEP { level });

zjit/src/hir/opt_tests.rs

@@ -1778,13 +1778,10 @@ mod hir_opt_tests {
           v16:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008))
           v17:StringExact = StringCopy v16
           v19:RangeExact = NewRange v14 NewRangeInclusive v17
-          v21:CPtr = GetEP 0
-          v23:CInt64 = LoadField v21, :_env_data_index_flags@0x1010
-          v24:CInt64 = GuardNoBitsSet v23, CUInt64(8)
           PatchPoint NoEPEscape(test)
-          v28:Fixnum[0] = Const Value(0)
+          v25:Fixnum[0] = Const Value(0)
           CheckInterrupts
-          Return v28
+          Return v25
         ");
     }
 
@@ -1838,13 +1835,10 @@ mod hir_opt_tests {
           Jump bb2(v5, v6)
         bb2(v8:BasicObject, v9:NilClass):
           v13:HashExact = NewHash
-          v15:CPtr = GetEP 0
-          v17:CInt64 = LoadField v15, :_env_data_index_flags@0x1000
-          v18:CInt64 = GuardNoBitsSet v17, CUInt64(8)
           PatchPoint NoEPEscape(test)
-          v22:Fixnum[5] = Const Value(5)
+          v19:Fixnum[5] = Const Value(5)
           CheckInterrupts
-          Return v22
+          Return v19
         ");
     }
 
@@ -1873,13 +1867,10 @@ mod hir_opt_tests {
           v19:StaticSymbol[:a] = Const Value(VALUE(0x1000))
           v22:StaticSymbol[:b] = Const Value(VALUE(0x1008))
           v25:HashExact = NewHash v19: v13, v22: v14
-          v27:CPtr = GetEP 0
-          v29:CInt64 = LoadField v27, :_env_data_index_flags@0x1010
-          v30:CInt64 = GuardNoBitsSet v29, CUInt64(8)
           PatchPoint NoEPEscape(test)
-          v34:Fixnum[5] = Const Value(5)
+          v31:Fixnum[5] = Const Value(5)
           CheckInterrupts
-          Return v34
+          Return v31
         ");
     }
 
@@ -2439,13 +2430,10 @@ mod hir_opt_tests {
           PatchPoint NoSingletonClass(Array@0x1000)
           PatchPoint MethodRedefined(Array@0x1000, itself@0x1008, cme:0x1010)
           IncrCounter inline_cfunc_optimized_send_count
-          v17:CPtr = GetEP 0
-          v19:CInt64 = LoadField v17, :_env_data_index_flags@0x1038
-          v20:CInt64 = GuardNoBitsSet v19, CUInt64(8)
           PatchPoint NoEPEscape(test)
-          v24:Fixnum[1] = Const Value(1)
+          v21:Fixnum[1] = Const Value(1)
           CheckInterrupts
-          Return v24
+          Return v21
         ");
     }
 
@@ -2473,18 +2461,15 @@ mod hir_opt_tests {
         bb2(v8:BasicObject, v9:NilClass):
           PatchPoint SingleRactorMode
           PatchPoint StableConstantNames(0x1000, M)
-          v32:ModuleExact[VALUE(0x1008)] = Const Value(VALUE(0x1008))
+          v29:ModuleExact[VALUE(0x1008)] = Const Value(VALUE(0x1008))
           PatchPoint NoSingletonClass(Module@0x1010)
           PatchPoint MethodRedefined(Module@0x1010, name@0x1018, cme:0x1020)
           IncrCounter inline_cfunc_optimized_send_count
-          v37:StringExact|NilClass = CCall v32, :Module#name@0x1048
-          v18:CPtr = GetEP 0
-          v20:CInt64 = LoadField v18, :_env_data_index_flags@0x1050
-          v21:CInt64 = GuardNoBitsSet v20, CUInt64(8)
+          v34:StringExact|NilClass = CCall v29, :Module#name@0x1048
           PatchPoint NoEPEscape(test)
-          v25:Fixnum[1] = Const Value(1)
+          v22:Fixnum[1] = Const Value(1)
           CheckInterrupts
-          Return v25
+          Return v22
         ");
     }
 
@@ -7536,18 +7521,15 @@ mod hir_opt_tests {
           Jump bb2(v5, v6)
         bb2(v8:BasicObject, v9:NilClass):
           v13:HashExact = NewHash
-          v15:CPtr = GetEP 0
-          v17:CInt64 = LoadField v15, :_env_data_index_flags@0x1000
-          v18:CInt64 = GuardNoBitsSet v17, CUInt64(8)
           PatchPoint NoEPEscape(test)
-          v25:Fixnum[1] = Const Value(1)
-          v27:Fixnum[3] = Const Value(3)
-          PatchPoint NoSingletonClass(Hash@0x1008)
-          PatchPoint MethodRedefined(Hash@0x1008, []=@0x1010, cme:0x1018)
-          HashAset v13, v25, v27
+          v22:Fixnum[1] = Const Value(1)
+          v24:Fixnum[3] = Const Value(3)
+          PatchPoint NoSingletonClass(Hash@0x1000)
+          PatchPoint MethodRedefined(Hash@0x1000, []=@0x1008, cme:0x1010)
+          HashAset v13, v22, v24
           IncrCounter inline_cfunc_optimized_send_count
           CheckInterrupts
-          Return v27
+          Return v24
         ");
     }
 

zjit/src/hir/tests.rs

@@ -2263,12 +2263,10 @@ pub mod hir_build_tests {
           PatchPoint BOPRedefined(ARRAY_REDEFINED_OP_FLAG, BOP_HASH)
           v33:Fixnum = ArrayHash v15, v16
           v35:CPtr = GetEP 0
-          v37:CInt64 = LoadField v35, :_env_data_index_flags@0x1000
-          v38:CInt64 = GuardNoBitsSet v37, CUInt64(8)
           PatchPoint NoEPEscape(test)
-          v43:ArrayExact[VALUE(0x1008)] = Const Value(VALUE(0x1008))
-          v44:ArrayExact = ArrayDup v43
-          v46:BasicObject = SendWithoutBlock v14, :puts, v44 # SendFallbackReason: Uncategorized(opt_send_without_block)
+          v40:ArrayExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
+          v41:ArrayExact = ArrayDup v40
+          v43:BasicObject = SendWithoutBlock v14, :puts, v41 # SendFallbackReason: Uncategorized(opt_send_without_block)
           PatchPoint NoEPEscape(test)
           CheckInterrupts
           Return v33
@@ -2462,12 +2460,10 @@ pub mod hir_build_tests {
           PatchPoint BOPRedefined(ARRAY_REDEFINED_OP_FLAG, BOP_INCLUDE_P)
           v34:BoolExact = ArrayInclude v15, v16 | v16
           v36:CPtr = GetEP 0
-          v38:CInt64 = LoadField v36, :_env_data_index_flags@0x1000
-          v39:CInt64 = GuardNoBitsSet v38, CUInt64(8)
           PatchPoint NoEPEscape(test)
-          v44:ArrayExact[VALUE(0x1008)] = Const Value(VALUE(0x1008))
-          v45:ArrayExact = ArrayDup v44
-          v47:BasicObject = SendWithoutBlock v14, :puts, v45 # SendFallbackReason: Uncategorized(opt_send_without_block)
+          v41:ArrayExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
+          v42:ArrayExact = ArrayDup v41
+          v44:BasicObject = SendWithoutBlock v14, :puts, v42 # SendFallbackReason: Uncategorized(opt_send_without_block)
           PatchPoint NoEPEscape(test)
           CheckInterrupts
           Return v34
@@ -3223,14 +3219,12 @@ pub mod hir_build_tests {
           PatchPoint SingleRactorMode
           v26:BasicObject = GetIvar v12, :@c
           v29:CPtr = GetEP 0
-          v31:CInt64 = LoadField v29, :_env_data_index_flags@0x1000
-          v32:CInt64 = GuardNoBitsSet v31, CUInt64(8)
           PatchPoint NoEPEscape(reverse_odd)
-          v35:CPtr = GetEP 0
-          v37:CPtr = GetEP 0
-          v43:ArrayExact = NewArray v20, v23, v26
+          v32:CPtr = GetEP 0
+          v34:CPtr = GetEP 0
+          v40:ArrayExact = NewArray v20, v23, v26
           CheckInterrupts
-          Return v43
+          Return v40
 
         fn reverse_even@<compiled>:8:
         bb0():
@@ -3258,15 +3252,13 @@ pub mod hir_build_tests {
           PatchPoint SingleRactorMode
           v32:BasicObject = GetIvar v14, :@d
           v35:CPtr = GetEP 0
-          v37:CInt64 = LoadField v35, :_env_data_index_flags@0x1000
-          v38:CInt64 = GuardNoBitsSet v37, CUInt64(8)
           PatchPoint NoEPEscape(reverse_even)
-          v41:CPtr = GetEP 0
-          v43:CPtr = GetEP 0
-          v45:CPtr = GetEP 0
-          v52:ArrayExact = NewArray v23, v26, v29, v32
+          v38:CPtr = GetEP 0
+          v40:CPtr = GetEP 0
+          v42:CPtr = GetEP 0
+          v49:ArrayExact = NewArray v23, v26, v29, v32
           CheckInterrupts
-          Return v52
+          Return v49
         ");
     }
 
@@ -3480,27 +3472,25 @@ pub mod hir_build_tests {
         bb2(v16:BasicObject, v17:BasicObject, v18:BasicObject, v19:BasicObject, v20:BasicObject, v21:NilClass):
           v25:BasicObject = InvokeBuiltin dir_s_open, v16, v17, v18
           v27:CPtr = GetEP 0
-          v29:CInt64 = LoadField v27, :_env_data_index_flags@0x1000
-          v30:CInt64 = GuardNoBitsSet v29, CUInt64(8)
           PatchPoint NoEPEscape(open)
-          v34:CPtr = GetEP 0
-          v35:CInt64 = LoadField v34, :_env_data_index_flags@0x1000
-          v36:CInt64 = GuardNoBitsSet v35, CUInt64(512)
-          v37:CInt64 = LoadField v34, :_env_data_index_specval@0x1001
-          v38:CInt64 = GuardAnyBitSet v37, CUInt64(1)
-          v39:HeapObject[BlockParamProxy] = Const Value(VALUE(0x1008))
-          CheckInterrupts
-          v42:CBool[true] = Test v39
-          v43 = RefineType v39, Falsy
-          IfFalse v42, bb3(v16, v17, v18, v19, v20, v25)
-          v45:HeapObject[BlockParamProxy] = RefineType v39, Truthy
-          v49:BasicObject = InvokeBlock, v25 # SendFallbackReason: Uncategorized(invokeblock)
-          v52:BasicObject = InvokeBuiltin dir_s_close, v16, v25
+          v31:CPtr = GetEP 0
+          v32:CInt64 = LoadField v31, :_env_data_index_flags@0x1000
+          v33:CInt64 = GuardNoBitsSet v32, CUInt64(512)
+          v34:CInt64 = LoadField v31, :_env_data_index_specval@0x1001
+          v35:CInt64 = GuardAnyBitSet v34, CUInt64(1)
+          v36:HeapObject[BlockParamProxy] = Const Value(VALUE(0x1008))
+          CheckInterrupts
+          v39:CBool[true] = Test v36
+          v40 = RefineType v36, Falsy
+          IfFalse v39, bb3(v16, v17, v18, v19, v20, v25)
+          v42:HeapObject[BlockParamProxy] = RefineType v36, Truthy
+          v46:BasicObject = InvokeBlock, v25 # SendFallbackReason: Uncategorized(invokeblock)
+          v49:BasicObject = InvokeBuiltin dir_s_close, v16, v25
           CheckInterrupts
-          Return v49
-        bb3(v58, v59, v60, v61, v62, v63):
+          Return v46
+        bb3(v55, v56, v57, v58, v59, v60):
           CheckInterrupts
-          Return v63
+          Return v60
         ");
     }
 
@@ -3890,10 +3880,8 @@ pub mod hir_build_tests {
           v26:CInt64[0] = Const CInt64(0)
           v27:BasicObject = ArrayAref v21, v26
           v29:CPtr = GetEP 0
-          v31:CInt64 = LoadField v29, :_env_data_index_flags@0x1000
-          v32:CInt64 = GuardNoBitsSet v31, CUInt64(8)
           PatchPoint NoEPEscape(test)
-          v35:CPtr = GetEP 0
+          v32:CPtr = GetEP 0
           CheckInterrupts
           Return v13
         ");