Skip to content

Commit

Permalink
nil fills
Browse files Browse the repository at this point in the history
  • Loading branch information
@XrXr
XrXr committed Aug 11, 2023
1 parent 08ad84c commit 72c9ee5
Showing 1 changed file with 54 additions and 51 deletions.
105 changes: 54 additions & 51 deletions yjit/src/codegen.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4952,7 +4952,6 @@ struct ControlFrame {
frame_type: u32,
specval: SpecVal,
cme: *const rb_callable_method_entry_t,
local_size: i32
}

// Codegen performing a similar (but not identical) function to vm_push_frame
Expand All @@ -4976,8 +4975,6 @@ fn gen_push_frame(
set_sp_cfp: bool, // if true CFP and SP will be switched to the callee
frame: ControlFrame,
) {
assert!(frame.local_size >= 0);

let sp = frame.sp;

asm.comment("push cme, specval, frame type");
Expand Down Expand Up @@ -5059,20 +5056,6 @@ fn gen_push_frame(
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_SELF), frame.recv);
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_BLOCK_CODE), 0.into());

// This Qnil fill snippet potentially requires 2 more registers on Arm, one for Qnil and
// another for calculating the address in case there are a lot of local variables. So doing
// this after releasing the register for specval and the receiver to avoid register spill.
let num_locals = frame.local_size;
if num_locals > 0 {
asm.comment("initialize locals");

// Initialize local variables to Qnil
for i in 0..num_locals {
let offs = SIZEOF_VALUE_I32 * (i - num_locals - 3);
asm.store(Opnd::mem(64, sp, offs), Qnil.into());
}
}

// Spill stack temps to let the callee use them (must be done before changing SP)
asm.spill_temps();

Expand Down Expand Up @@ -5304,7 +5287,6 @@ fn gen_send_cfunc(
None // Leave PC uninitialized as cfuncs shouldn't read it
},
iseq: None,
local_size: 0,
});

if !kw_arg.is_null() {
Expand Down Expand Up @@ -5623,6 +5605,8 @@ fn gen_send_iseq(
argc: i32,
captured_opnd: Option<Opnd>,
) -> Option<CodegenStatus> {
// XXX: expand on this with the various identities you use
// The argument deepest in the stack will become the 0th local in the callee.
let mut argc = argc;

// When you have keyword arguments, there is an extra object that gets
Expand Down Expand Up @@ -6041,7 +6025,7 @@ fn gen_send_iseq(
new_ary
};

// Find where the rest parameter array should go
// Find where to put the rest parameter array
let rest_param = if opts_missing == 0 {
// All optionals are filled, the rest param goes at the top of the stack
argc += 1;
Expand All @@ -6064,37 +6048,6 @@ fn gen_send_iseq(
asm.store(rest_param, rest_param_array);
}

// Nil-initialize missing optional parameters in case they are not
// contiguous with non-parameter locals. When there is no gap between
// the two they'll be nil-initilaized with non-parameter locals later.
if opts_missing > 0 &&
required_num + opt_num != unsafe { get_iseq_body_param_size(iseq) as c_int } {
asm.comment("nil fill missing opt params");

// rest + unfilled opts?

// because kw unspecificed bits comes after rest
// if opts_missing == 0 -> push
// else opts_missing >0 -> don't? nah
// ^ wrong because either way kw uses -1.

// add to stack size, if appropriate
// set opts_missing to all unknown - or nil?
// actually, no, because only argc get type transferred in the context
// and the unspecified bits below use stack_opnd(-1)

// XXX: ^ contradiction between the two.
// ✅seems fine. because of exit_if_doing_kw_and_opts_missing
// and exit_if_doing_kw_and_splat. So we never have the clobbering case of
// negative idx in the rest array movement above and the storing of
// unspecified_bit at stack_opnd(-1) below.
//
// XXX: wanting to place rest param at just the right spot without movement

// The argument deepest in the stack will become the first local in the callee.
let callee_locals_base = argc - 1;
}

if doing_kw_call {
// Here we're calling a method with keyword arguments and specifying
// keyword arguments at this call site.
Expand Down Expand Up @@ -6262,6 +6215,57 @@ fn gen_send_iseq(
argc = lead_num;
}

// because kw unspecificed bits comes after rest
// if opts_missing == 0 -> push
// else opts_missing >0 -> don't? nah
// ^ wrong because either way kw uses -1.

// add to stack size, if appropriate
// set opts_missing to all unknown - or nil?
// actually, no, because only argc get type transferred in the context
// and the unspecified bits below use stack_opnd(-1)

// XXX: ^ contradiction between the two.
// ✅seems fine. because of `exit_if_doing_kw_and_opts_missing`
// and `exit_if_doing_kw_and_splat`. So we never have the clobbering case of
// negative idx in the rest array movement above and the storing of
// unspecified_bit at stack_opnd(-1) below.

fn nil_fill(comment: &'static str, fill_range: std::ops::Range<isize>, asm: &mut Assembler) {
if fill_range.is_empty() {
return;
}

asm.comment(comment);
for i in fill_range {
let value_slot = asm.ctx.sp_opnd(i * SIZEOF_VALUE as isize);
asm.store(value_slot, Qnil.into());
}
}

// Nil-initialize missing optional parameters
nil_fill(
"nil-initialize missing optionals",
{
let begin = -(argc as isize) + required_num as isize + opts_filled as isize;
let end = -(argc as isize) + required_num as isize + opt_num as isize;

begin..end
},
asm
);
// Nil-initialize non-parameter local variables
nil_fill(
"nil-initialize locals",
{
let begin = -(argc as isize) + num_params as isize;
let end = -(argc as isize) + num_locals_total as isize;

begin..end
},
asm
);

// Points to the receiver operand on the stack unless a captured environment is used
let recv = match captured_opnd {
Some(captured_opnd) => asm.load(Opnd::mem(64, captured_opnd, 0)), // captured->self
Expand Down Expand Up @@ -6308,7 +6312,6 @@ fn gen_send_iseq(
sp: callee_sp,
iseq: Some(iseq),
pc: None, // We are calling into jitted code, which will set the PC as necessary
local_size: num_locals
});

// No need to set cfp->pc since the callee sets it whenever calling into routines
Expand Down

0 comments on commit 72c9ee5

Please sign in to comment.