Skip to content

Commit

Permalink
Support explicit endianness in Cranelift IR MemFlags
Browse files Browse the repository at this point in the history
WebAssembly memory operations are by definition little-endian even on
big-endian target platforms.  However, other memory accesses will require
native target endianness (e.g. to access parts of the VMContext that is
also accessed by VM native code).  This means on big-endian targets,
the code generator will have to handle both little- and big-endian
memory accesses.  However, there is currently no way to encode that
distinction into the Cranelift IR that describes memory accesses.

This patch provides such a way by adding an (optional) explicit
endianness marker to an instance of MemFlags.  Since each Cranelift IR
instruction that describes memory accesses already has an instance of
MemFlags attached, this can now be used to provide endianness
information.

Note that by default, memory accesses will continue to use the native
target ISA endianness.  To override this to specify an explicit
endianness, a MemFlags value that was built using the set_endianness
routine must be used.  This patch does so for accesses that implement
WebAssembly memory operations.

This patch addresses issue bytecodealliance#2124.
  • Loading branch information
uweigand authored and jlb6740 committed Dec 15, 2020
1 parent 2d7c2fb commit 4f87d7f
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 12 deletions.
54 changes: 51 additions & 3 deletions cranelift/codegen/src/ir/memflags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,31 @@ enum FlagBit {
Notrap,
Aligned,
Readonly,
LittleEndian,
BigEndian,
}

const NAMES: [&str; 3] = ["notrap", "aligned", "readonly"];
const NAMES: [&str; 5] = ["notrap", "aligned", "readonly", "little", "big"];

/// Endianness of a memory access.
#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
pub enum Endianness {
/// Little-endian
Little,
/// Big-endian
Big,
}

/// Flags for memory operations like load/store.
///
/// Each of these flags introduce a limited form of undefined behavior. The flags each enable
/// certain optimizations that need to make additional assumptions. Generally, the semantics of a
/// program does not change when a flag is removed, but adding a flag will.
///
/// In addition, the flags determine the endianness of the memory access. By default,
/// any memory access uses the native endianness determined by the target ISA. This can
/// be overridden for individual accesses by explicitly specifying little- or big-endian
/// semantics via the flags.
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
pub struct MemFlags {
bits: u8,
Expand Down Expand Up @@ -48,16 +64,48 @@ impl MemFlags {
/// Set a flag bit by name.
///
/// Returns true if the flag was found and set, false for an unknown flag name.
/// Will also return false when trying to set inconsistent endianness flags.
pub fn set_by_name(&mut self, name: &str) -> bool {
match NAMES.iter().position(|&s| s == name) {
Some(bit) => {
self.bits |= 1 << bit;
true
let bits = self.bits | 1 << bit;
if (bits & (1 << FlagBit::LittleEndian as usize)) != 0
&& (bits & (1 << FlagBit::BigEndian as usize)) != 0
{
false
} else {
self.bits = bits;
true
}
}
None => false,
}
}

/// Return endianness of the memory access. This will return the endianness
/// explicitly specified by the flags if any, and will default to the native
/// endianness otherwise. The native endianness has to be provided by the
/// caller since it is not explicitly encoded in CLIF IR -- this allows a
/// front end to create IR without having to know the target endianness.
pub fn endianness(self, native_endianness: Endianness) -> Endianness {
if self.read(FlagBit::LittleEndian) {
Endianness::Little
} else if self.read(FlagBit::BigEndian) {
Endianness::Big
} else {
native_endianness
}
}

/// Set endianness of the memory access.
pub fn set_endianness(&mut self, endianness: Endianness) {
match endianness {
Endianness::Little => self.set(FlagBit::LittleEndian),
Endianness::Big => self.set(FlagBit::BigEndian),
};
assert!(!(self.read(FlagBit::LittleEndian) && self.read(FlagBit::BigEndian)));
}

/// Test if the `notrap` flag is set.
///
/// Normally, trapping is part of the semantics of a load/store operation. If the platform
Expand Down
2 changes: 1 addition & 1 deletion cranelift/codegen/src/ir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub use crate::ir::instructions::{
pub use crate::ir::jumptable::JumpTableData;
pub use crate::ir::layout::Layout;
pub use crate::ir::libcall::{get_probestack_funcref, LibCall};
pub use crate::ir::memflags::MemFlags;
pub use crate::ir::memflags::{Endianness, MemFlags};
pub use crate::ir::progpoint::{ExpandedProgramPoint, ProgramOrder, ProgramPoint};
pub use crate::ir::sourceloc::SourceLoc;
pub use crate::ir::stackslot::{StackLayoutInfo, StackSlotData, StackSlotKind, StackSlots};
Expand Down
8 changes: 8 additions & 0 deletions cranelift/codegen/src/isa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,14 @@ pub trait TargetIsa: fmt::Display + Send + Sync {
CallConv::triple_default(self.triple())
}

/// Get the endianness of this ISA.
fn endianness(&self) -> ir::Endianness {
match self.triple().endianness().unwrap() {
target_lexicon::Endianness::Little => ir::Endianness::Little,
target_lexicon::Endianness::Big => ir::Endianness::Big,
}
}

/// Get the pointer type of this ISA.
fn pointer_type(&self) -> ir::Type {
ir::Type::int(u16::from(self.pointer_bits())).unwrap()
Expand Down
12 changes: 10 additions & 2 deletions cranelift/codegen/src/legalizer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ fn narrow_load(
inst: ir::Inst,
func: &mut ir::Function,
_cfg: &mut ControlFlowGraph,
_isa: &dyn TargetIsa,
isa: &dyn TargetIsa,
) {
let mut pos = FuncCursor::new(func).at_inst(inst);
pos.use_srcloc(inst);
Expand All @@ -684,6 +684,10 @@ fn narrow_load(
ptr,
offset.try_add_i64(8).expect("load offset overflow"),
);
let (al, ah) = match flags.endianness(isa.endianness()) {
ir::Endianness::Little => (al, ah),
ir::Endianness::Big => (ah, al),
};
pos.func.dfg.replace(inst).iconcat(al, ah);
}

Expand All @@ -692,7 +696,7 @@ fn narrow_store(
inst: ir::Inst,
func: &mut ir::Function,
_cfg: &mut ControlFlowGraph,
_isa: &dyn TargetIsa,
isa: &dyn TargetIsa,
) {
let mut pos = FuncCursor::new(func).at_inst(inst);
pos.use_srcloc(inst);
Expand All @@ -708,6 +712,10 @@ fn narrow_store(
};

let (al, ah) = pos.ins().isplit(val);
let (al, ah) = match flags.endianness(isa.endianness()) {
ir::Endianness::Little => (al, ah),
ir::Endianness::Big => (ah, al),
};
pos.ins().store(flags, al, ptr, offset);
pos.ins().store(
flags,
Expand Down
19 changes: 13 additions & 6 deletions cranelift/wasm/src/code_translator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2056,7 +2056,9 @@ fn prepare_load<FE: FuncEnvironment + ?Sized>(
// Note that we don't set `is_aligned` here, even if the load instruction's
// alignment immediate says it's aligned, because WebAssembly's immediate
// field is just a hint, while Cranelift's aligned flag needs a guarantee.
let flags = MemFlags::new();
// WebAssembly memory accesses are always little-endian.
let mut flags = MemFlags::new();
flags.set_endianness(ir::Endianness::Little);

Ok((flags, base, offset.into()))
}
Expand Down Expand Up @@ -2103,7 +2105,8 @@ fn translate_store<FE: FuncEnvironment + ?Sized>(
builder,
);
// See the comments in `prepare_load` about the flags.
let flags = MemFlags::new();
let mut flags = MemFlags::new();
flags.set_endianness(ir::Endianness::Little);
builder
.ins()
.Store(opcode, val_ty, flags, offset.into(), val, base);
Expand Down Expand Up @@ -2207,7 +2210,8 @@ fn translate_atomic_rmw<FE: FuncEnvironment + ?Sized>(
finalise_atomic_mem_addr(linear_mem_addr, memarg, access_ty, builder, state, environ)?;

// See the comments in `prepare_load` about the flags.
let flags = MemFlags::new();
let mut flags = MemFlags::new();
flags.set_endianness(ir::Endianness::Little);
let mut res = builder
.ins()
.atomic_rmw(access_ty, flags, op, final_effective_address, arg2);
Expand Down Expand Up @@ -2260,7 +2264,8 @@ fn translate_atomic_cas<FE: FuncEnvironment + ?Sized>(
finalise_atomic_mem_addr(linear_mem_addr, memarg, access_ty, builder, state, environ)?;

// See the comments in `prepare_load` about the flags.
let flags = MemFlags::new();
let mut flags = MemFlags::new();
flags.set_endianness(ir::Endianness::Little);
let mut res = builder
.ins()
.atomic_cas(flags, final_effective_address, expected, replacement);
Expand Down Expand Up @@ -2302,7 +2307,8 @@ fn translate_atomic_load<FE: FuncEnvironment + ?Sized>(
finalise_atomic_mem_addr(linear_mem_addr, memarg, access_ty, builder, state, environ)?;

// See the comments in `prepare_load` about the flags.
let flags = MemFlags::new();
let mut flags = MemFlags::new();
flags.set_endianness(ir::Endianness::Little);
let mut res = builder
.ins()
.atomic_load(access_ty, flags, final_effective_address);
Expand Down Expand Up @@ -2348,7 +2354,8 @@ fn translate_atomic_store<FE: FuncEnvironment + ?Sized>(
finalise_atomic_mem_addr(linear_mem_addr, memarg, access_ty, builder, state, environ)?;

// See the comments in `prepare_load` about the flags.
let flags = MemFlags::new();
let mut flags = MemFlags::new();
flags.set_endianness(ir::Endianness::Little);
builder
.ins()
.atomic_store(flags, data, final_effective_address);
Expand Down

0 comments on commit 4f87d7f

Please sign in to comment.