Skip to content

Commit

Permalink
Don't force veneers on island emission (#6902)
Browse files Browse the repository at this point in the history
* Don't force veneers on island emission

This commit is a follow-up to #6804 with the discussion on #6818. This
undoes some of the changes from #6804, such as bringing a size parameter
back to `emit_island`, and implements the changes discussed in #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 #6818

* Keep a running count for pending fixup deadline

Update this as pending fixups are added and then clear this out when
islands are emitted.

* Don't force all fixups to go through a binary heap

Instead process them immediately if they're ready.
  • Loading branch information
alexcrichton authored Aug 24, 2023
1 parent e17b6c8 commit f8c03d5
Show file tree
Hide file tree
Showing 9 changed files with 213 additions and 221 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 f8c03d5

Please sign in to comment.