Skip to content

Commit

Permalink
A bunch of minor cleanups (#6767)
Browse files Browse the repository at this point in the history
* Remove DisplayFunctionAnnotations

It used to exist for printing the debuginfo value ranges with the clif
ir, but this no longer happens, so it is now useless.

* Remove debug info collection from DummyEnvironment

There are no remaining users of it

* Remove ComparableSourceLoc

It is unused

* Move LabelValueLoc re-export out of the ir module

It encodes target specific information, so shouldn't be in the target
independent ir module.

* Remove RelocDistance dependency from ir::extfunc and ir::globalvalue
  • Loading branch information
bjorn3 authored Jul 25, 2023
1 parent e81af41 commit 729e264
Show file tree
Hide file tree
Showing 17 changed files with 52 additions and 174 deletions.
10 changes: 0 additions & 10 deletions cranelift/codegen/src/ir/extfunc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

use crate::ir::{ExternalName, SigRef, Type};
use crate::isa::CallConv;
use crate::machinst::RelocDistance;
use alloc::vec::Vec;
use core::fmt;
use core::str::FromStr;
Expand Down Expand Up @@ -319,15 +318,6 @@ pub struct ExtFuncData {
}

impl ExtFuncData {
/// Return an estimate of the distance to the referred-to function symbol.
pub fn reloc_distance(&self) -> RelocDistance {
if self.colocated {
RelocDistance::Near
} else {
RelocDistance::Far
}
}

/// Returns a displayable version of the `ExtFuncData`, with or without extra context to
/// prettify the output.
pub fn display<'a>(
Expand Down
22 changes: 3 additions & 19 deletions cranelift/codegen/src/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use crate::ir::{
StackSlots, Table, TableData, Type,
};
use crate::isa::CallConv;
use crate::value_label::ValueLabelsRanges;
use crate::write::write_function;
use crate::HashMap;
#[cfg(feature = "enable-serde")]
Expand Down Expand Up @@ -417,15 +416,7 @@ impl Function {

/// Return an object that can display this function with correct ISA-specific annotations.
pub fn display(&self) -> DisplayFunction<'_> {
DisplayFunction(self, Default::default())
}

/// Return an object that can display this function with correct ISA-specific annotations.
pub fn display_with<'a>(
&'a self,
annotations: DisplayFunctionAnnotations<'a>,
) -> DisplayFunction<'a> {
DisplayFunction(self, annotations)
DisplayFunction(self)
}

/// Sets an absolute source location for the given instruction.
Expand Down Expand Up @@ -456,15 +447,8 @@ impl Function {
}
}

/// Additional annotations for function display.
#[derive(Default)]
pub struct DisplayFunctionAnnotations<'a> {
/// Enable value labels annotations.
pub value_ranges: Option<&'a ValueLabelsRanges>,
}

/// Wrapper type capable of displaying a `Function` with correct ISA annotations.
pub struct DisplayFunction<'a>(&'a Function, DisplayFunctionAnnotations<'a>);
/// Wrapper type capable of displaying a `Function`.
pub struct DisplayFunction<'a>(&'a Function);

impl<'a> fmt::Display for DisplayFunction<'a> {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
Expand Down
15 changes: 0 additions & 15 deletions cranelift/codegen/src/ir/globalvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use crate::ir::immediates::{Imm64, Offset32};
use crate::ir::{ExternalName, GlobalValue, Type};
use crate::isa::TargetIsa;
use crate::machinst::RelocDistance;
use core::fmt;

#[cfg(feature = "enable-serde")]
Expand Down Expand Up @@ -102,20 +101,6 @@ impl GlobalValueData {
Self::DynScaleTargetConst { .. } => isa.pointer_type(),
}
}

/// If this global references a symbol, return an estimate of the relocation distance,
/// based on the `colocated` flag.
pub fn maybe_reloc_distance(&self) -> Option<RelocDistance> {
match self {
&GlobalValueData::Symbol {
colocated: true, ..
} => Some(RelocDistance::Near),
&GlobalValueData::Symbol {
colocated: false, ..
} => Some(RelocDistance::Far),
_ => None,
}
}
}

impl fmt::Display for GlobalValueData {
Expand Down
3 changes: 1 addition & 2 deletions cranelift/codegen/src/ir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub use crate::ir::extfunc::{
AbiParam, ArgumentExtension, ArgumentPurpose, ExtFuncData, Signature,
};
pub use crate::ir::extname::{ExternalName, UserExternalName, UserFuncName};
pub use crate::ir::function::{DisplayFunctionAnnotations, Function};
pub use crate::ir::function::Function;
pub use crate::ir::globalvalue::GlobalValueData;
pub use crate::ir::instructions::{
BlockCall, InstructionData, Opcode, ValueList, ValueListPool, VariableArgs,
Expand All @@ -62,7 +62,6 @@ pub use crate::ir::stackslot::{
pub use crate::ir::table::TableData;
pub use crate::ir::trapcode::TrapCode;
pub use crate::ir::types::Type;
pub use crate::value_label::LabelValueLoc;

use crate::entity::{entity_impl, PrimaryMap, SecondaryMap};

Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use hashbrown::{hash_map, HashMap, HashSet};
use std::collections::{hash_map, HashMap, HashSet};

pub use crate::context::Context;
pub use crate::value_label::{ValueLabelsRanges, ValueLocRange};
pub use crate::value_label::{LabelValueLoc, ValueLabelsRanges, ValueLocRange};
pub use crate::verifier::verify_function;
pub use crate::write::write_function;

Expand Down
11 changes: 6 additions & 5 deletions cranelift/codegen/src/machinst/isle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,11 +306,12 @@ macro_rules! isle_lower_prelude_methods {
#[inline]
fn func_ref_data(&mut self, func_ref: FuncRef) -> (SigRef, ExternalName, RelocDistance) {
let funcdata = &self.lower_ctx.dfg().ext_funcs[func_ref];
(
funcdata.signature,
funcdata.name.clone(),
funcdata.reloc_distance(),
)
let reloc_distance = if funcdata.colocated {
RelocDistance::Near
} else {
RelocDistance::Far
};
(funcdata.signature, funcdata.name.clone(), reloc_distance)
}

#[inline]
Expand Down
7 changes: 6 additions & 1 deletion cranelift/codegen/src/machinst/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1120,10 +1120,15 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
&GlobalValueData::Symbol {
ref name,
ref offset,
colocated,
..
} => {
let offset = offset.bits();
let dist = gvdata.maybe_reloc_distance().unwrap();
let dist = if colocated {
RelocDistance::Near
} else {
RelocDistance::Far
};
Some((name, dist, offset))
}
_ => None,
Expand Down
4 changes: 2 additions & 2 deletions cranelift/codegen/src/machinst/vcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@
use crate::fx::FxHashMap;
use crate::fx::FxHashSet;
use crate::ir::RelSourceLoc;
use crate::ir::{self, types, Constant, ConstantData, DynamicStackSlot, LabelValueLoc, ValueLabel};
use crate::ir::{self, types, Constant, ConstantData, DynamicStackSlot, ValueLabel};
use crate::machinst::*;
use crate::timing;
use crate::trace;
use crate::CodegenError;
use crate::ValueLocRange;
use crate::{LabelValueLoc, ValueLocRange};
use cranelift_control::ControlPlane;
use regalloc2::{
Edit, Function as RegallocFunction, InstOrEdit, InstRange, Operand, OperandKind, PRegSet,
Expand Down
39 changes: 1 addition & 38 deletions cranelift/codegen/src/value_label.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use crate::ir::{SourceLoc, ValueLabel};
use crate::ir::ValueLabel;
use crate::machinst::Reg;
use crate::HashMap;
use alloc::vec::Vec;
use core::cmp::Ordering;
use core::convert::From;
use core::ops::Deref;

#[cfg(feature = "enable-serde")]
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -33,37 +30,3 @@ pub enum LabelValueLoc {

/// Resulting map of Value labels and their ranges/locations.
pub type ValueLabelsRanges = HashMap<ValueLabel, Vec<ValueLocRange>>;

#[derive(Eq, Clone, Copy)]
pub struct ComparableSourceLoc(SourceLoc);

impl From<SourceLoc> for ComparableSourceLoc {
fn from(s: SourceLoc) -> Self {
Self(s)
}
}

impl Deref for ComparableSourceLoc {
type Target = SourceLoc;
fn deref(&self) -> &SourceLoc {
&self.0
}
}

impl PartialOrd for ComparableSourceLoc {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}

impl Ord for ComparableSourceLoc {
fn cmp(&self, other: &Self) -> Ordering {
self.0.bits().cmp(&other.0.bits())
}
}

impl PartialEq for ComparableSourceLoc {
fn eq(&self, other: &Self) -> bool {
self.0 == other.0
}
}
3 changes: 0 additions & 3 deletions cranelift/filetests/src/test_wasm/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ use std::collections::BTreeMap;

#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct TestConfig {
#[serde(default)]
pub debug_info: bool,

#[serde(default)]
pub target: String,

Expand Down
6 changes: 1 addition & 5 deletions cranelift/filetests/src/test_wasm/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub struct ModuleEnv {

impl ModuleEnv {
pub fn new(target_isa: &dyn TargetIsa, config: TestConfig) -> Self {
let inner = DummyEnvironment::new(target_isa.frontend_config(), config.debug_info);
let inner = DummyEnvironment::new(target_isa.frontend_config());
Self {
inner,
config,
Expand Down Expand Up @@ -64,10 +64,6 @@ impl<'data> ModuleEnvironment<'data> for ModuleEnv {
sig,
);

if self.inner.debug_info {
func.collect_debug_info();
}

self.inner
.trans
.translate_body(&mut validator, body, &mut func, &mut func_environ)?;
Expand Down
2 changes: 1 addition & 1 deletion cranelift/src/souper_harvest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub fn run(options: &Options) -> Result<()> {
.context("failed to read input file")?;

let funcs = if &contents[..WASM_MAGIC.len()] == WASM_MAGIC {
let mut dummy_environ = DummyEnvironment::new(fisa.isa.unwrap().frontend_config(), false);
let mut dummy_environ = DummyEnvironment::new(fisa.isa.unwrap().frontend_config());
cranelift_wasm::translate_module(&contents, &mut dummy_environ)
.context("failed to translate Wasm module to clif")?;
dummy_environ
Expand Down
20 changes: 2 additions & 18 deletions cranelift/src/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use crate::disasm::print_all;
use anyhow::{Context as _, Result};
use clap::Parser;
use cranelift_codegen::ir::DisplayFunctionAnnotations;
use cranelift_codegen::print_errors::{pretty_error, pretty_verifier_error};
use cranelift_codegen::settings::FlagsOrIsa;
use cranelift_codegen::timing;
Expand Down Expand Up @@ -102,10 +101,6 @@ pub struct Options {
#[clap(short = 'c')]
check_translation: bool,

/// Display values' ranges and their locations
#[clap(long = "value-ranges")]
value_ranges: bool,

/// Use colors in output? [options: auto/never/always; default: auto]
#[clap(long = "color", default_value("auto"))]
color: ColorOpt,
Expand Down Expand Up @@ -185,8 +180,7 @@ fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) -
}
};

let debug_info = options.value_ranges;
let mut dummy_environ = DummyEnvironment::new(isa.frontend_config(), debug_info);
let mut dummy_environ = DummyEnvironment::new(isa.frontend_config());
translate_module(&module_binary, &mut dummy_environ)?;

vcprintln!(options.verbose, use_color, terminal, Color::Green, "ok");
Expand Down Expand Up @@ -296,17 +290,7 @@ fn handle_module(options: &Options, path: &Path, name: &str, fisa: FlagsOrIsa) -
{
println!("; Exported as \"{}\"", export_name);
}
let value_ranges = if options.value_ranges {
Some(context.compiled_code().unwrap().value_labels_ranges.clone())
} else {
None
};
println!(
"{}",
context.func.display_with(DisplayFunctionAnnotations {
value_ranges: value_ranges.as_ref(),
})
);
println!("{}", context.func.display());
vprintln!(options.verbose, "");
}

Expand Down
10 changes: 1 addition & 9 deletions cranelift/wasm/src/environ/dummy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,6 @@ pub struct DummyEnvironment {
/// Vector of wasm bytecode size for each function.
pub func_bytecode_sizes: Vec<usize>,

/// Instructs to collect debug data during translation.
pub debug_info: bool,

/// Name of the module from the wasm file.
pub module_name: Option<String>,

Expand All @@ -160,12 +157,11 @@ pub struct DummyEnvironment {

impl DummyEnvironment {
/// Creates a new `DummyEnvironment` instance.
pub fn new(config: TargetFrontendConfig, debug_info: bool) -> Self {
pub fn new(config: TargetFrontendConfig) -> Self {
Self {
info: DummyModuleInfo::new(config),
trans: FuncTranslator::new(),
func_bytecode_sizes: Vec::new(),
debug_info,
module_name: None,
function_names: SecondaryMap::new(),
expected_reachability: None,
Expand Down Expand Up @@ -905,10 +901,6 @@ impl<'data> ModuleEnvironment<'data> for DummyEnvironment {
let mut func =
ir::Function::with_name_signature(UserFuncName::user(0, func_index.as_u32()), sig);

if self.debug_info {
func.collect_debug_info();
}

self.trans
.translate_body(&mut validator, body, &mut func, &mut func_environ)?;
func
Expand Down
33 changes: 12 additions & 21 deletions cranelift/wasm/src/func_translator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,10 @@ mod tests {

let mut trans = FuncTranslator::new();
let flags = settings::Flags::new(settings::builder());
let runtime = DummyEnvironment::new(
isa::TargetFrontendConfig {
default_call_conv: isa::CallConv::Fast,
pointer_width: PointerWidth::U64,
},
false,
);
let runtime = DummyEnvironment::new(isa::TargetFrontendConfig {
default_call_conv: isa::CallConv::Fast,
pointer_width: PointerWidth::U64,
});

let mut ctx = Context::new();

Expand Down Expand Up @@ -360,13 +357,10 @@ mod tests {

let mut trans = FuncTranslator::new();
let flags = settings::Flags::new(settings::builder());
let runtime = DummyEnvironment::new(
isa::TargetFrontendConfig {
default_call_conv: isa::CallConv::Fast,
pointer_width: PointerWidth::U64,
},
false,
);
let runtime = DummyEnvironment::new(isa::TargetFrontendConfig {
default_call_conv: isa::CallConv::Fast,
pointer_width: PointerWidth::U64,
});

let mut ctx = Context::new();

Expand Down Expand Up @@ -403,13 +397,10 @@ mod tests {

let mut trans = FuncTranslator::new();
let flags = settings::Flags::new(settings::builder());
let runtime = DummyEnvironment::new(
isa::TargetFrontendConfig {
default_call_conv: isa::CallConv::Fast,
pointer_width: PointerWidth::U64,
},
false,
);
let runtime = DummyEnvironment::new(isa::TargetFrontendConfig {
default_call_conv: isa::CallConv::Fast,
pointer_width: PointerWidth::U64,
});

let mut ctx = Context::new();

Expand Down
Loading

0 comments on commit 729e264

Please sign in to comment.