Skip to content

Commit

Permalink
Don't force veneers on island emission
Browse files Browse the repository at this point in the history
This commit is a follow-up to bytecodealliance#6804 with the discussion on bytecodealliance#6818. This
undoes some of the changes from bytecodealliance#6804, such as bringing a size parameter
back to `emit_island`, and implements the changes discussed in bytecodealliance#6818.
Namely fixups are now tracked in a `pending_fixups` list for editing and
modification and then during island emission they're flushed into a
`BinaryHeap` tracked as `fixup_records`. Additionally calculation of the
size of the pending island is now done as a single calculation rather
than updating metadata as we go along. This is required because fixups
are no longer entirely cleared during island emission so the previous
logic of "reset the island size to zero" and have it get recalculated as
the island is emitted is no longer applicable. I opted to go with a
simplistic version of this for now which assumes that all veneers are
the worst case size, but if necessary I think this could be more optimal
by tracking each veneer in a counter.

Closes bytecodealliance#6818
  • Loading branch information
alexcrichton committed Aug 24, 2023
1 parent 55fb2ee commit 4602e71
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 171 deletions.
4 changes: 2 additions & 2 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3579,7 +3579,7 @@ impl MachInstEmit for Inst {
dest: BranchTarget::Label(jump_around_label),
};
jmp.emit(&[], sink, emit_info, state);
sink.emit_island(&mut state.ctrl_plane);
sink.emit_island(needed_space + 4, &mut state.ctrl_plane);
sink.bind_label(jump_around_label, &mut state.ctrl_plane);
}
}
Expand Down Expand Up @@ -3776,7 +3776,7 @@ fn emit_return_call_common_sequence(
dest: BranchTarget::Label(jump_around_label),
};
jmp.emit(&[], sink, emit_info, state);
sink.emit_island(&mut state.ctrl_plane);
sink.emit_island(space_needed + 4, &mut state.ctrl_plane);
sink.bind_label(jump_around_label, &mut state.ctrl_plane);
}

Expand Down
4 changes: 4 additions & 0 deletions cranelift/codegen/src/isa/aarch64/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2984,6 +2984,10 @@ impl MachInstLabelUse for LabelUse {
}
}

fn worst_case_veneer_size() -> CodeOffset {
20
}

/// Generate a veneer into the buffer, given that this veneer is at `veneer_offset`, and return
/// an offset and label-use for the veneer's use of the original label.
fn generate_veneer(
Expand Down
4 changes: 2 additions & 2 deletions cranelift/codegen/src/isa/riscv64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@ impl MachInstEmit for Inst {
// we need to emit a jump table here to support that jump.
let distance = (targets.len() * 2 * Inst::INSTRUCTION_SIZE as usize) as u32;
if sink.island_needed(distance) {
sink.emit_island(&mut state.ctrl_plane);
sink.emit_island(distance, &mut state.ctrl_plane);
}

// Emit the jumps back to back
Expand Down Expand Up @@ -3104,7 +3104,7 @@ fn emit_return_call_common_sequence(
dest: BranchTarget::Label(jump_around_label),
}
.emit(&[], sink, emit_info, state);
sink.emit_island(&mut state.ctrl_plane);
sink.emit_island(space_needed + 4, &mut state.ctrl_plane);
sink.bind_label(jump_around_label, &mut state.ctrl_plane);
}

Expand Down
4 changes: 4 additions & 0 deletions cranelift/codegen/src/isa/riscv64/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2014,6 +2014,10 @@ impl MachInstLabelUse for LabelUse {
}
}

fn worst_case_veneer_size() -> CodeOffset {
8
}

/// Generate a veneer into the buffer, given that this veneer is at `veneer_offset`, and return
/// an offset and label-use for the veneer's use of the original label.
fn generate_veneer(
Expand Down
4 changes: 4 additions & 0 deletions cranelift/codegen/src/isa/s390x/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3406,6 +3406,10 @@ impl MachInstLabelUse for LabelUse {
0
}

fn worst_case_veneer_size() -> CodeOffset {
0
}

/// Generate a veneer into the buffer, given that this veneer is at `veneer_offset`, and return
/// an offset and label-use for the veneer's use of the original label.
fn generate_veneer(
Expand Down
4 changes: 4 additions & 0 deletions cranelift/codegen/src/isa/x64/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2742,6 +2742,10 @@ impl MachInstLabelUse for LabelUse {
}
}

fn worst_case_veneer_size() -> CodeOffset {
0
}

fn generate_veneer(self, _: &mut [u8], _: CodeOffset) -> (CodeOffset, LabelUse) {
match self {
LabelUse::JmpRel32 | LabelUse::PCRel32 => {
Expand Down
Loading

0 comments on commit 4602e71

Please sign in to comment.