ZJIT: Don't mark control-flow opcodes as invalidating locals (ruby#15694)

jump, branchif, etc don't invalidate locals in the JIT; they might in the interpreter because they can execute arbitrary code, but the JIT side exits before that happens.

Author: tekknolagi

zjit/src/hir.rs

@@ -5299,6 +5299,20 @@ impl ProfileOracle {
     }
 }
 
+fn invalidates_locals(opcode: u32, operands: *const VALUE) -> bool {
+    match opcode {
+        // Control-flow is non-leaf in the interpreter because it can execute arbitrary code on
+        // interrupt. But in the JIT, we side-exit if there is a pending interrupt.
+        YARVINSN_jump
+        | YARVINSN_branchunless
+        | YARVINSN_branchif
+        | YARVINSN_branchnil
+        | YARVINSN_leave => false,
+        // TODO(max): Read the invokebuiltin target from operands and determine if it's leaf
+        _ => unsafe { !rb_zjit_insn_leaf(opcode as i32, operands) }
+    }
+}
+
 /// The index of the self parameter in the HIR function
 pub const SELF_PARAM_IDX: usize = 0;
 
@@ -5434,7 +5448,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
             }
 
             // Flag a future getlocal/setlocal to add a patch point if this instruction is not leaf.
-            if unsafe { !rb_zjit_insn_leaf(opcode as i32, pc.offset(1)) } {
+            if invalidates_locals(opcode, unsafe { pc.offset(1) }) {
                 local_inval = true;
             }
 

zjit/src/hir/opt_tests.rs

@@ -9638,26 +9638,24 @@ mod hir_opt_tests {
          Jump bb2(v8, v9, v10, v11, v12)
        bb2(v14:BasicObject, v15:BasicObject, v16:BasicObject, v17:BasicObject, v18:NilClass):
          CheckInterrupts
-         v27:BasicObject = GetLocal :a, l0, EP@6
-         SetLocal :formatted, l0, EP@3, v27
-         v39:BasicObject = GetLocal :formatted, l0, EP@3
+         SetLocal :formatted, l0, EP@3, v15
          PatchPoint SingleRactorMode
-         v56:HeapBasicObject = GuardType v14, HeapBasicObject
-         v57:HeapBasicObject = GuardShape v56, 0x1000
-         StoreField v57, :@formatted@0x1001, v39
-         WriteBarrier v57, v39
-         v60:CShape[0x1002] = Const CShape(0x1002)
-         StoreField v57, :_shape_id@0x1003, v60
-         v45:Class[VMFrozenCore] = Const Value(VALUE(0x1008))
+         v54:HeapBasicObject = GuardType v14, HeapBasicObject
+         v55:HeapBasicObject = GuardShape v54, 0x1000
+         StoreField v55, :@formatted@0x1001, v15
+         WriteBarrier v55, v15
+         v58:CShape[0x1002] = Const CShape(0x1002)
+         StoreField v55, :_shape_id@0x1003, v58
+         v43:Class[VMFrozenCore] = Const Value(VALUE(0x1008))
          PatchPoint MethodRedefined(Class@0x1010, lambda@0x1018, cme:0x1020)
          PatchPoint NoSingletonClass(Class@0x1010)
-         v65:BasicObject = CCallWithFrame v45, :RubyVM::FrozenCore.lambda@0x1048, block=0x1050
-         v48:BasicObject = GetLocal :a, l0, EP@6
-         v49:BasicObject = GetLocal :_b, l0, EP@5
-         v50:BasicObject = GetLocal :_c, l0, EP@4
-         v51:BasicObject = GetLocal :formatted, l0, EP@3
+         v63:BasicObject = CCallWithFrame v43, :RubyVM::FrozenCore.lambda@0x1048, block=0x1050
+         v46:BasicObject = GetLocal :a, l0, EP@6
+         v47:BasicObject = GetLocal :_b, l0, EP@5
+         v48:BasicObject = GetLocal :_c, l0, EP@4
+         v49:BasicObject = GetLocal :formatted, l0, EP@3
          CheckInterrupts
-         Return v65
+         Return v63
        ");
     }
 

zjit/src/hir/tests.rs

@@ -1069,17 +1069,14 @@ pub mod hir_build_tests {
           v18:CBool = Test v11
           IfFalse v18, bb3(v10, v11, v12)
           v22:Fixnum[3] = Const Value(3)
-          PatchPoint NoEPEscape(test)
           CheckInterrupts
           Jump bb4(v10, v11, v22)
-        bb3(v29:BasicObject, v30:BasicObject, v31:NilClass):
-          v35:Fixnum[4] = Const Value(4)
-          PatchPoint NoEPEscape(test)
-          Jump bb4(v29, v30, v35)
-        bb4(v40:BasicObject, v41:BasicObject, v42:Fixnum):
-          PatchPoint NoEPEscape(test)
+        bb3(v27:BasicObject, v28:BasicObject, v29:NilClass):
+          v33:Fixnum[4] = Const Value(4)
+          Jump bb4(v27, v28, v33)
+        bb4(v36:BasicObject, v37:BasicObject, v38:Fixnum):
           CheckInterrupts
-          Return v42
+          Return v38
         ");
     }
 
@@ -1366,23 +1363,20 @@ pub mod hir_build_tests {
           CheckInterrupts
           Jump bb4(v10, v16, v20)
         bb4(v26:BasicObject, v27:BasicObject, v28:BasicObject):
-          PatchPoint NoEPEscape(test)
-          v34:Fixnum[0] = Const Value(0)
-          v37:BasicObject = SendWithoutBlock v28, :>, v34 # SendFallbackReason: Uncategorized(opt_gt)
+          v32:Fixnum[0] = Const Value(0)
+          v35:BasicObject = SendWithoutBlock v28, :>, v32 # SendFallbackReason: Uncategorized(opt_gt)
           CheckInterrupts
-          v40:CBool = Test v37
-          IfTrue v40, bb3(v26, v27, v28)
-          v43:NilClass = Const Value(nil)
-          PatchPoint NoEPEscape(test)
+          v38:CBool = Test v35
+          IfTrue v38, bb3(v26, v27, v28)
+          v41:NilClass = Const Value(nil)
           CheckInterrupts
           Return v27
-        bb3(v53:BasicObject, v54:BasicObject, v55:BasicObject):
-          PatchPoint NoEPEscape(test)
-          v62:Fixnum[1] = Const Value(1)
-          v65:BasicObject = SendWithoutBlock v54, :+, v62 # SendFallbackReason: Uncategorized(opt_plus)
-          v70:Fixnum[1] = Const Value(1)
-          v73:BasicObject = SendWithoutBlock v55, :-, v70 # SendFallbackReason: Uncategorized(opt_minus)
-          Jump bb4(v53, v65, v73)
+        bb3(v49:BasicObject, v50:BasicObject, v51:BasicObject):
+          v56:Fixnum[1] = Const Value(1)
+          v59:BasicObject = SendWithoutBlock v50, :+, v56 # SendFallbackReason: Uncategorized(opt_plus)
+          v64:Fixnum[1] = Const Value(1)
+          v67:BasicObject = SendWithoutBlock v51, :-, v64 # SendFallbackReason: Uncategorized(opt_minus)
+          Jump bb4(v49, v59, v67)
         ");
     }
 
@@ -3064,15 +3058,13 @@ pub mod hir_build_tests {
           CheckInterrupts
           v35:CBool[true] = Test v32
           IfFalse v35, bb3(v16, v17, v18, v19, v20, v25)
-          PatchPoint NoEPEscape(open)
-          v42:BasicObject = InvokeBlock, v25 # SendFallbackReason: Uncategorized(invokeblock)
-          v45:BasicObject = InvokeBuiltin dir_s_close, v16, v25
+          v40:BasicObject = InvokeBlock, v25 # SendFallbackReason: Uncategorized(invokeblock)
+          v43:BasicObject = InvokeBuiltin dir_s_close, v16, v25
           CheckInterrupts
-          Return v42
-        bb3(v51, v52, v53, v54, v55, v56):
-          PatchPoint NoEPEscape(open)
+          Return v40
+        bb3(v49, v50, v51, v52, v53, v54):
           CheckInterrupts
-          Return v56
+          Return v54
         ");
     }
 
@@ -3548,12 +3540,10 @@ pub mod hir_build_tests {
           v22:Fixnum[1] = Const Value(1)
           v24:Fixnum[1] = Const Value(1)
           v27:BasicObject = SendWithoutBlock v22, :+, v24 # SendFallbackReason: Uncategorized(opt_plus)
-          PatchPoint NoEPEscape(test)
           Jump bb3(v10, v27, v12)
-        bb3(v32:BasicObject, v33:BasicObject, v34:BasicObject):
-          PatchPoint NoEPEscape(test)
+        bb3(v30:BasicObject, v31:BasicObject, v32:BasicObject):
           CheckInterrupts
-          Return v33
+          Return v31
         ");
     }