ZJIT: Put keyword bits in callee frame rather than c_args

Author: rwstauner

zjit/src/codegen.rs

@@ -384,9 +384,6 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
         // Give up SendWithoutBlockDirect for 6+ args since asm.ccall() doesn't support it.
         Insn::SendWithoutBlockDirect { cd, state, args, .. } if args.len() + 1 > C_ARG_OPNDS.len() => // +1 for self
             gen_send_without_block(jit, asm, *cd, &function.frame_state(*state), SendFallbackReason::TooManyArgsForLir),
-        // Give up SendWithoutBlockDirect for 5+ args (plus 1 arg for keyword bits) since asm.ccall() doesn't support it.
-        Insn::SendWithoutBlockDirect { cd, state, args, iseq, .. } if args.len() + 2 > C_ARG_OPNDS.len() && unsafe { rb_get_iseq_flags_has_kw(*iseq) } => // +1 for self +1 for keyword bits
-            gen_send_without_block(jit, asm, *cd, &function.frame_state(*state), SendFallbackReason::TooManyArgsForLir),
         Insn::SendWithoutBlockDirect { cme, iseq, recv, args, state, .. } => gen_send_without_block_direct(cb, jit, asm, *cme, *iseq, opnd!(recv), opnds!(args), &function.frame_state(*state)),
         &Insn::InvokeSuper { cd, blockiseq, state, reason, .. } => gen_invokesuper(jit, asm, cd, blockiseq, &function.frame_state(state), reason),
         &Insn::InvokeBlock { cd, state, reason, .. } => gen_invokeblock(jit, asm, cd, &function.frame_state(state), reason),
@@ -1368,13 +1365,6 @@ fn gen_send_without_block_direct(
     let mut c_args = vec![recv];
     c_args.extend(&args);
 
-    if unsafe { rb_get_iseq_flags_has_kw(iseq) } {
-        // Currently we only get to this point if all the accepted keyword args are required.
-        let unspecified_bits = 0;
-        // For each optional keyword that isn't passed we would `unspecified_bits |= (0x01 << idx)`.
-        c_args.push(VALUE::fixnum_from_usize(unspecified_bits).into());
-    }
-
     let params = unsafe { iseq.params() };
     let num_optionals_passed = if params.flags.has_opt() != 0 {
         // See vm_call_iseq_setup_normal_opt_start in vm_inshelper.c

zjit/src/hir.rs

@@ -6239,12 +6239,29 @@ fn compile_jit_entry_state(fun: &mut Function, jit_entry_block: BlockId, jit_ent
     let lead_num: usize = params.lead_num.try_into().expect("iseq param lead_num >= 0");
     let passed_opt_num = jit_entry_idx;
 
+    // If the iseq has keyword parameters, the keyword bits local will be appended to the local table.
+    let kw_bits_idx: Option<usize> = if unsafe { rb_get_iseq_flags_has_kw(iseq) } {
+        let keyword = unsafe { rb_get_iseq_body_param_keyword(iseq) };
+        if !keyword.is_null() {
+            Some(unsafe { (*keyword).bits_start } as usize)
+        } else {
+            None
+        }
+    } else {
+        None
+    };
+
     let self_param = fun.push_insn(jit_entry_block, Insn::Param);
     let mut entry_state = FrameState::new(iseq);
     for local_idx in 0..num_locals(iseq) {
         if (lead_num + passed_opt_num..lead_num + opt_num).contains(&local_idx) {
             // Omitted optionals are locals, so they start as nils before their code run
             entry_state.locals.push(fun.push_insn(jit_entry_block, Insn::Const { val: Const::Value(Qnil) }));
+        } else if Some(local_idx) == kw_bits_idx {
+            // We currently only support required keywords so the unspecified bits will always be zero.
+            // TODO: Make this a parameter when we start writing anything other than zero.
+            let unspecified_bits = VALUE::fixnum_from_usize(0);
+            entry_state.locals.push(fun.push_insn(jit_entry_block, Insn::Const { val: Const::Value(unspecified_bits) }));
         } else if local_idx < param_size {
             entry_state.locals.push(fun.push_insn(jit_entry_block, Insn::Param));
         } else {

zjit/src/hir/opt_tests.rs

@@ -626,8 +626,9 @@ mod hir_opt_tests {
           v2:BasicObject = GetLocal :k, l0, SP@5
           v3:BasicObject = GetLocal <empty>, l0, SP@4
           Jump bb2(v1, v2, v3)
-        bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject):
+        bb1(v6:BasicObject, v7:BasicObject):
           EntryPoint JIT(0)
+          v8:Fixnum[0] = Const Value(0)
           Jump bb2(v6, v7, v8)
         bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject):
           CheckInterrupts

zjit/src/hir/tests.rs

@@ -3000,8 +3000,9 @@ pub mod hir_build_tests {
           v3:BasicObject = GetLocal :exception, l0, SP@5
           v4:BasicObject = GetLocal <empty>, l0, SP@4
           Jump bb2(v1, v2, v3, v4)
-        bb1(v7:BasicObject, v8:BasicObject, v9:BasicObject, v10:BasicObject):
+        bb1(v7:BasicObject, v8:BasicObject, v9:BasicObject):
           EntryPoint JIT(0)
+          v10:Fixnum[0] = Const Value(0)
           Jump bb2(v7, v8, v9, v10)
         bb2(v12:BasicObject, v13:BasicObject, v14:BasicObject, v15:BasicObject):
           v19:Float = InvokeBuiltin rb_f_float, v12, v13, v14
@@ -3050,8 +3051,9 @@ pub mod hir_build_tests {
           v5:BasicObject = GetLocal :block, l0, SP@5
           v6:NilClass = Const Value(nil)
           Jump bb2(v1, v2, v3, v4, v5, v6)
-        bb1(v9:BasicObject, v10:BasicObject, v11:BasicObject, v12:BasicObject, v13:BasicObject):
+        bb1(v9:BasicObject, v10:BasicObject, v11:BasicObject, v13:BasicObject):
           EntryPoint JIT(0)
+          v12:Fixnum[0] = Const Value(0)
           v14:NilClass = Const Value(nil)
           Jump bb2(v9, v10, v11, v12, v13, v14)
         bb2(v16:BasicObject, v17:BasicObject, v18:BasicObject, v19:BasicObject, v20:BasicObject, v21:NilClass):
@@ -3112,8 +3114,9 @@ pub mod hir_build_tests {
           v4:BasicObject = GetLocal :immediate_sweep, l0, SP@5
           v5:BasicObject = GetLocal <empty>, l0, SP@4
           Jump bb2(v1, v2, v3, v4, v5)
-        bb1(v8:BasicObject, v9:BasicObject, v10:BasicObject, v11:BasicObject, v12:BasicObject):
+        bb1(v8:BasicObject, v9:BasicObject, v10:BasicObject, v11:BasicObject):
           EntryPoint JIT(0)
+          v12:Fixnum[0] = Const Value(0)
           Jump bb2(v8, v9, v10, v11, v12)
         bb2(v14:BasicObject, v15:BasicObject, v16:BasicObject, v17:BasicObject, v18:BasicObject):
           v25:FalseClass = Const Value(false)
@@ -3532,8 +3535,9 @@ pub mod hir_build_tests {
           v2:BasicObject = GetLocal :kw, l0, SP@5
           v3:BasicObject = GetLocal <empty>, l0, SP@4
           Jump bb2(v1, v2, v3)
-        bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject):
+        bb1(v6:BasicObject, v7:BasicObject):
           EntryPoint JIT(0)
+          v8:Fixnum[0] = Const Value(0)
           Jump bb2(v6, v7, v8)
         bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject):
           v15:BasicObject = GetLocal <empty>, l0, EP@3
@@ -3605,8 +3609,9 @@ pub mod hir_build_tests {
           v34:BasicObject = GetLocal :k33, l0, SP@5
           v35:BasicObject = GetLocal <empty>, l0, SP@4
           Jump bb2(v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11, v12, v13, v14, v15, v16, v17, v18, v19, v20, v21, v22, v23, v24, v25, v26, v27, v28, v29, v30, v31, v32, v33, v34, v35)
-        bb1(v38:BasicObject, v39:BasicObject, v40:BasicObject, v41:BasicObject, v42:BasicObject, v43:BasicObject, v44:BasicObject, v45:BasicObject, v46:BasicObject, v47:BasicObject, v48:BasicObject, v49:BasicObject, v50:BasicObject, v51:BasicObject, v52:BasicObject, v53:BasicObject, v54:BasicObject, v55:BasicObject, v56:BasicObject, v57:BasicObject, v58:BasicObject, v59:BasicObject, v60:BasicObject, v61:BasicObject, v62:BasicObject, v63:BasicObject, v64:BasicObject, v65:BasicObject, v66:BasicObject, v67:BasicObject, v68:BasicObject, v69:BasicObject, v70:BasicObject, v71:BasicObject, v72:BasicObject):
+        bb1(v38:BasicObject, v39:BasicObject, v40:BasicObject, v41:BasicObject, v42:BasicObject, v43:BasicObject, v44:BasicObject, v45:BasicObject, v46:BasicObject, v47:BasicObject, v48:BasicObject, v49:BasicObject, v50:BasicObject, v51:BasicObject, v52:BasicObject, v53:BasicObject, v54:BasicObject, v55:BasicObject, v56:BasicObject, v57:BasicObject, v58:BasicObject, v59:BasicObject, v60:BasicObject, v61:BasicObject, v62:BasicObject, v63:BasicObject, v64:BasicObject, v65:BasicObject, v66:BasicObject, v67:BasicObject, v68:BasicObject, v69:BasicObject, v70:BasicObject, v71:BasicObject):
           EntryPoint JIT(0)
+          v72:Fixnum[0] = Const Value(0)
           Jump bb2(v38, v39, v40, v41, v42, v43, v44, v45, v46, v47, v48, v49, v50, v51, v52, v53, v54, v55, v56, v57, v58, v59, v60, v61, v62, v63, v64, v65, v66, v67, v68, v69, v70, v71, v72)
         bb2(v74:BasicObject, v75:BasicObject, v76:BasicObject, v77:BasicObject, v78:BasicObject, v79:BasicObject, v80:BasicObject, v81:BasicObject, v82:BasicObject, v83:BasicObject, v84:BasicObject, v85:BasicObject, v86:BasicObject, v87:BasicObject, v88:BasicObject, v89:BasicObject, v90:BasicObject, v91:BasicObject, v92:BasicObject, v93:BasicObject, v94:BasicObject, v95:BasicObject, v96:BasicObject, v97:BasicObject, v98:BasicObject, v99:BasicObject, v100:BasicObject, v101:BasicObject, v102:BasicObject, v103:BasicObject, v104:BasicObject, v105:BasicObject, v106:BasicObject, v107:BasicObject, v108:BasicObject):
           SideExit TooManyKeywordParameters