Skip to content

Commit b9bb4e8

Browse files
philipcbongjunj
authored andcommitted
Only emit value label aliases for lowered instructions (bytecodealliance#12779)
If we emit a value label alias for an instruction that isn't lowered, then that signals that the value has been optimised out. However, since it is an alias we know that the value also exists in an earlier vreg, so we should skip the alias and use that instead. This situation occurs often for memory indexes on AArch64. We translate memory stores into instructions such as: v8 = iconst.i32 42 v9 = uextend.i64 v6 v10 = load.i64 notrap aligned readonly can_move checked v0+56 v11 = iadd v10, v9 v12 = iconst.i64 20 v13 = iadd v11, v12 ; v12 = 20 store little heap v8, v13 ; v8 = 42 Here, v6 is a memory index (which has a label) and v9 is an extension of the memory index (which has a label alias, added by cast_index_to_pointer_ty()). This is lowered to: 40c: 52800540 mov w0, #0x2a // bytecodealliance#42 410: f9401c41 ldr x1, [x2, bytecodealliance#56] 414: 91005021 add x1, x1, #0x14 418: b8384820 str w0, [x1, w24, uxtw] The uextend has been folded into the str, so v9 has been optimised out. But v6 is still present in w24, so the debuginfo should use that instead. This fixes the following tests for AArch64: native_debug::lldb::dwarf_cold_block native_debug::lldb::dwarf_fib_wasm native_debug::lldb::dwarf_fib_wasm_dwarf5 native_debug::lldb::dwarf_fib_wasm_split4 native_debug::lldb::dwarf_fission native_debug::lldb::dwarf_fraction_norm native_debug::lldb::dwarf_imported_memory native_debug::lldb::dwarf_shared_memory native_debug::lldb::dwarf_simple native_debug::lldb::dwarf_spilled_frame_base
1 parent 4321e38 commit b9bb4e8

1 file changed

Lines changed: 8 additions & 6 deletions

File tree

cranelift/codegen/src/machinst/lower.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,9 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
802802

803803
// Value defined by "inst" becomes live after it in normal
804804
// order, and therefore **before** in reversed order.
805-
self.emit_value_label_live_range_start_for_inst(inst);
805+
// Only emit value label aliases if the instruction will be lowered
806+
// (otherwise we want to keep using the earlier label instead).
807+
self.emit_value_label_live_range_start_for_inst(inst, has_side_effect || value_needed);
806808

807809
// Normal instruction: codegen if the instruction is side-effecting
808810
// or any of its outputs is used.
@@ -949,14 +951,14 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
949951
}
950952
}
951953

952-
fn emit_value_label_marks_for_value(&mut self, val: Value) {
954+
fn emit_value_label_marks_for_value(&mut self, val: Value, allow_alias: bool) {
953955
let regs = self.value_regs[val];
954956
if regs.len() > 1 {
955957
return;
956958
}
957959
let reg = regs.only_reg().unwrap();
958960

959-
if let Some(label_starts) = self.get_value_labels(val, 0) {
961+
if let Some(label_starts) = self.get_value_labels(val, if allow_alias { 0 } else { !0 }) {
960962
let labels = label_starts
961963
.iter()
962964
.map(|&ValueLabelStart { label, .. }| label)
@@ -971,7 +973,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
971973
}
972974
}
973975

974-
fn emit_value_label_live_range_start_for_inst(&mut self, inst: Inst) {
976+
fn emit_value_label_live_range_start_for_inst(&mut self, inst: Inst, allow_alias: bool) {
975977
if self.f.dfg.values_labels.is_none() {
976978
return;
977979
}
@@ -982,7 +984,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
982984
inst
983985
);
984986
for &val in self.f.dfg.inst_results(inst) {
985-
self.emit_value_label_marks_for_value(val);
987+
self.emit_value_label_marks_for_value(val, allow_alias);
986988
}
987989
}
988990

@@ -993,7 +995,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
993995

994996
trace!("value labeling: block {}", block);
995997
for &arg in self.f.dfg.block_params(block) {
996-
self.emit_value_label_marks_for_value(arg);
998+
self.emit_value_label_marks_for_value(arg, true);
997999
}
9981000
self.finish_ir_inst(Default::default());
9991001
}

0 commit comments

Comments
 (0)