Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A bunch of minor cleanups #6767

Merged
merged 5 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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