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

x64: avoid load-coalescing SIMD operations with non-aligned loads #3107

Merged
merged 3 commits into from
Jul 26, 2021
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
1 change: 1 addition & 0 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ fn main() -> anyhow::Result<()> {
test_directory_module(out, "tests/misc_testsuite/reference-types", strategy)?;
test_directory_module(out, "tests/misc_testsuite/multi-memory", strategy)?;
test_directory_module(out, "tests/misc_testsuite/module-linking", strategy)?;
test_directory_module(out, "tests/misc_testsuite/simd", strategy)?;
test_directory_module(out, "tests/misc_testsuite/threads", strategy)?;
Ok(())
})?;
Expand Down
24 changes: 20 additions & 4 deletions cranelift/codegen/src/ir/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@ use core::str::FromStr;
#[cfg(feature = "enable-serde")]
use serde::{Deserialize, Serialize};

use crate::ir::{self, trapcode::TrapCode, types, Block, FuncRef, JumpTable, SigRef, Type, Value};
use crate::isa;

use crate::bitset::BitSet;
use crate::data_value::DataValue;
use crate::entity;
use ir::condcodes::{FloatCC, IntCC};
use crate::ir::{
self,
condcodes::{FloatCC, IntCC},
trapcode::TrapCode,
types, Block, FuncRef, JumpTable, MemFlags, SigRef, Type, Value,
};
use crate::isa;

/// Some instructions use an external list of argument values because there is not enough space in
/// the 16-byte `InstructionData` struct. These value lists are stored in a memory pool in
Expand Down Expand Up @@ -395,6 +398,19 @@ impl InstructionData {
}
}

/// If this is a load/store instruction, return its memory flags.
pub fn memflags(&self) -> Option<MemFlags> {
match self {
&InstructionData::Load { flags, .. }
| &InstructionData::LoadComplex { flags, .. }
| &InstructionData::LoadNoOffset { flags, .. }
| &InstructionData::Store { flags, .. }
| &InstructionData::StoreComplex { flags, .. }
| &InstructionData::StoreNoOffset { flags, .. } => Some(flags),
_ => None,
}
}

/// Return information about a call instruction.
///
/// Any instruction that can call another function reveals its call signature here.
Expand Down
6 changes: 6 additions & 0 deletions cranelift/codegen/src/isa/x64/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ fn is_mergeable_load<C: LowerCtx<I = Inst>>(
return None;
}

// SIMD instructions can only be load-coalesced when the loaded value comes
// from an aligned address.
if load_ty.is_vector() && !insn_data.memflags().map_or(false, |f| f.aligned()) {
return None;
}

// Just testing the opcode is enough, because the width will always match if
// the type does (and the type should match if the CLIF is properly
// constructed).
Expand Down
13 changes: 13 additions & 0 deletions tests/misc_testsuite/simd/unaligned-load.wast
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
(; See discussion at https://github.com/bytecodealliance/wasmtime/issues/2943 ;)
(module
(memory 1)
(data (i32.const 1) "\01\00\00\00\01\00\00\00")

(func $unaligned_load (export "unaligned_load") (result v128)
v128.const i32x4 0 0 1 1
i32.const 1
v128.load
v128.xor)
)

(assert_return (invoke "unaligned_load") (v128.const i32x4 1 1 1 1))