Skip to content

Commit

Permalink
Auto merge of #117124 - bjorn3:sync_cg_clif-2023-10-24, r=bjorn3
Browse files Browse the repository at this point in the history
Sync rustc_codegen_cranelift

This contains fixes for the last two remaining known miscompilations. One is the lack of stack alignment support in cranelift which has been worked around by dynamically realigning at runtime. This fixed rayon and by extension Wasmtime. And the other is lack of zero/sign extending of small arguments when the ABI requires this. This is completely fine when only using cg_clif compiled code, but LLVM depends on this resulting in weird behavior of mixed LLVM, Cranelift binaries. The update to Cranelift 0.101.1 fixes this. In addition I have implemented all x86_64 SIMD intrinsics required by the image and rav1e crates.

r? `@ghost`

`@rustbot` label +A-codegen +A-cranelift +T-compiler
  • Loading branch information
bors committed Oct 24, 2023
2 parents cee6db1 + 484bc7f commit 642bfb2
Show file tree
Hide file tree
Showing 14 changed files with 260 additions and 138 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/.vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
]
},
{
"sysroot_src": "./download/sysroot/sysroot_src/library",
"sysroot_src": "./build/stdlib/library",
"crates": [
{
"root_module": "./example/std_example.rs",
Expand Down
52 changes: 26 additions & 26 deletions compiler/rustc_codegen_cranelift/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,18 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"

[[package]]
name = "cranelift-bforest"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8e5e1df0da8488dd03b34afc134ba84b754d61862cc465932a9e5d07952f661e"
checksum = "c1512c3bb6b13018e7109fc3ac964bc87b329eaf3a77825d337558d0c7f6f1be"
dependencies = [
"cranelift-entity",
]

[[package]]
name = "cranelift-codegen"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "77a17ca4e699a0aaf49a0c88f6311a864f321048aa63f6b787cab20eb5f93f10"
checksum = "16cb8fb9220a6ea7a226705a273ab905309ee546267bdf34948d57932d7f0396"
dependencies = [
"bumpalo",
"cranelift-bforest",
Expand All @@ -75,39 +75,39 @@ dependencies = [

[[package]]
name = "cranelift-codegen-meta"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "022f2793cdade1d37a1f755ac42938a3f832f533eac6cafc8b26b209544c3c06"
checksum = "ab3a8d3b0d4745b183da5ea0792b13d79f5c23d6e69ac04761728e2532b56649"
dependencies = [
"cranelift-codegen-shared",
]

[[package]]
name = "cranelift-codegen-shared"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a4d72dbb83c2ad788dec4ad0843070973cb48c35a3ca19b1e7437ac40834fd9c"
checksum = "524141c8e68f2abc2043de4c2b31f6d9dd42432738c246431d0572a1422a4a84"

[[package]]
name = "cranelift-control"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ae07cf26dcc90d546826d747ac63b6c40c916f34b03e92a6ae0422c28d771b8a"
checksum = "97513b57c961c713789a03886a57b43e14ebcd204cbaa8ae50ca6c70a8e716b3"
dependencies = [
"arbitrary",
]

[[package]]
name = "cranelift-entity"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c2fe6b7e49820893691aea497f36257e9d6f52061d8c4758d61d802d5f101a3d"
checksum = "e3f23d3cf3afa7e45f239702612c76d87964f652a55e28d13ed6d7e20f3479dd"

[[package]]
name = "cranelift-frontend"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "44f497576ca3674581581601b6a55ccc1b43447217648c880e5bce70db3cf659"
checksum = "554cd4947ec9209b58bf9ae5bf83581b5ddf9128bd967208e334b504a57db54e"
dependencies = [
"cranelift-codegen",
"log",
Expand All @@ -117,15 +117,15 @@ dependencies = [

[[package]]
name = "cranelift-isle"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b96aa02eac00fffee13b0cd37d17874ccdb3d5458983041accd825ef78ce6454"
checksum = "6c1892a439696b6413cb54083806f5fd9fc431768b8de74864b3d9e8b93b124f"

[[package]]
name = "cranelift-jit"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b1d6e0e308c873eefc185745a6b21daec2a10f7554c9fb67e334c2d7d756d979"
checksum = "32209252fb38acaf1662ccd0397907bbe0e92bdb13b6ddbfd2f74e437f83e685"
dependencies = [
"anyhow",
"cranelift-codegen",
Expand All @@ -143,9 +143,9 @@ dependencies = [

[[package]]
name = "cranelift-module"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c1aa8ebb06eced4e478c3f94f1d65d4e7c93493f4640057912b27a3e34b84841"
checksum = "bf42656f5f6df7bfafc4dd7b63a1888b0627c07b43b2cb9aa54e13843fed39eb"
dependencies = [
"anyhow",
"cranelift-codegen",
Expand All @@ -154,9 +154,9 @@ dependencies = [

[[package]]
name = "cranelift-native"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2870170ca44054b202c737626607b87be6e35655084bd94a6ff807a5812ba7df"
checksum = "e0c2d3badd4b9690865f5bb68a71fa94de592fa2df3f3d11a5a062c60c0a107a"
dependencies = [
"cranelift-codegen",
"libc",
Expand All @@ -165,9 +165,9 @@ dependencies = [

[[package]]
name = "cranelift-object"
version = "0.101.0"
version = "0.101.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "20647761742d17dabac8205da958910ede78599550e06418a16711a3ee2fc897"
checksum = "88eca54bbecea3170035168357306e9c779d4a63d8bf036c9e16bd21fdaa69b5"
dependencies = [
"anyhow",
"cranelift-codegen",
Expand Down Expand Up @@ -374,9 +374,9 @@ checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"

[[package]]
name = "wasmtime-jit-icache-coherence"
version = "14.0.0"
version = "14.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a3a5dda53ad6993f9b0a2d65fb49e0348a7232a27a8794064122870d6ee19eb2"
checksum = "9aaf2fa8fd2d6b65abae9b92edfe69254cc5d6b166e342364036c3e347de8da9"
dependencies = [
"cfg-if",
"libc",
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_codegen_cranelift/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ crate-type = ["dylib"]

[dependencies]
# These have to be in sync with each other
cranelift-codegen = { version = "0.101", features = ["unwind", "all-arch"] }
cranelift-frontend = { version = "0.101" }
cranelift-module = { version = "0.101" }
cranelift-native = { version = "0.101" }
cranelift-jit = { version = "0.101", optional = true }
cranelift-object = { version = "0.101" }
cranelift-codegen = { version = "0.101.1", features = ["unwind", "all-arch"] }
cranelift-frontend = { version = "0.101.1" }
cranelift-module = { version = "0.101.1" }
cranelift-native = { version = "0.101.1" }
cranelift-jit = { version = "0.101.1", optional = true }
cranelift-object = { version = "0.101.1" }
target-lexicon = "0.12.0"
gimli = { version = "0.28", default-features = false, features = ["write"]}
object = { version = "0.32", default-features = false, features = ["std", "read_core", "write", "archive", "coff", "elf", "macho", "pe"] }
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_cranelift/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ If not please open an issue.
## Building and testing

```bash
$ git clone https://github.com/bjorn3/rustc_codegen_cranelift
$ git clone https://github.com/rust-lang/rustc_codegen_cranelift
$ cd rustc_codegen_cranelift
$ ./y.sh prepare
$ ./y.sh build
Expand All @@ -29,7 +29,7 @@ Extract the `dist` directory in the archive anywhere you want.
If you want to use `cargo clif build` instead of having to specify the full path to the `cargo-clif` executable, you can add the `bin` subdirectory of the extracted `dist` directory to your `PATH`.
(tutorial [for Windows](https://stackoverflow.com/a/44272417), and [for Linux/MacOS](https://unix.stackexchange.com/questions/26047/how-to-correctly-add-a-path-to-path/26059#26059)).

[releases]: https://github.com/bjorn3/rustc_codegen_cranelift/releases/tag/dev
[releases]: https://github.com/rust-lang/rustc_codegen_cranelift/releases/tag/dev

## Usage

Expand Down Expand Up @@ -78,7 +78,7 @@ configuration options.

* Inline assembly ([no cranelift support](https://github.com/bytecodealliance/wasmtime/issues/1041))
* On UNIX there is support for invoking an external assembler for `global_asm!` and `asm!`.
* SIMD ([tracked here](https://github.com/bjorn3/rustc_codegen_cranelift/issues/171), `std::simd` fully works, `std::arch` is partially supported)
* SIMD ([tracked here](https://github.com/rust-lang/rustc_codegen_cranelift/issues/171), `std::simd` fully works, `std::arch` is partially supported)
* Unwinding on panics ([no cranelift support](https://github.com/bytecodealliance/wasmtime/issues/1677), `-Cpanic=abort` is enabled by default)

## License
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_codegen_cranelift/example/mini_core_hello_world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,17 @@ fn main() {

let f = V([0.0, 1.0]);
let _a = f.0[0];

stack_val_align();
}

#[inline(never)]
fn stack_val_align() {
#[repr(align(8192))]
struct Foo(u8);

let a = Foo(0);
assert_eq!(&a as *const Foo as usize % 8192, 0);
}

#[cfg(all(
Expand Down

This file was deleted.

35 changes: 14 additions & 21 deletions compiler/rustc_codegen_cranelift/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,32 +120,25 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
args: &[Value],
) -> Cow<'_, [Value]> {
if self.tcx.sess.target.is_like_windows {
let (mut params, mut args): (Vec<_>, Vec<_>) =
params
.into_iter()
.zip(args)
.map(|(param, &arg)| {
if param.value_type == types::I128 {
let arg_ptr = Pointer::stack_slot(self.bcx.create_sized_stack_slot(
StackSlotData { kind: StackSlotKind::ExplicitSlot, size: 16 },
));
arg_ptr.store(self, arg, MemFlags::trusted());
(AbiParam::new(self.pointer_type), arg_ptr.get_addr(self))
} else {
(param, arg)
}
})
.unzip();
let (mut params, mut args): (Vec<_>, Vec<_>) = params
.into_iter()
.zip(args)
.map(|(param, &arg)| {
if param.value_type == types::I128 {
let arg_ptr = self.create_stack_slot(16, 16);
arg_ptr.store(self, arg, MemFlags::trusted());
(AbiParam::new(self.pointer_type), arg_ptr.get_addr(self))
} else {
(param, arg)
}
})
.unzip();

let indirect_ret_val = returns.len() == 1 && returns[0].value_type == types::I128;

if indirect_ret_val {
params.insert(0, AbiParam::new(self.pointer_type));
let ret_ptr =
Pointer::stack_slot(self.bcx.create_sized_stack_slot(StackSlotData {
kind: StackSlotKind::ExplicitSlot,
size: 16,
}));
let ret_ptr = self.create_stack_slot(16, 16);
args.insert(0, ret_ptr.get_addr(self));
self.lib_call_unadjusted(name, params, vec![], &args);
return Cow::Owned(vec![ret_ptr.load(self, types::I128, MemFlags::trusted())]);
Expand Down
11 changes: 4 additions & 7 deletions compiler/rustc_codegen_cranelift/src/abi/pass_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,16 +189,13 @@ pub(super) fn from_casted_value<'tcx>(
let abi_params = cast_target_to_abi_params(cast);
let abi_param_size: u32 = abi_params.iter().map(|param| param.value_type.bytes()).sum();
let layout_size = u32::try_from(layout.size.bytes()).unwrap();
let stack_slot = fx.bcx.create_sized_stack_slot(StackSlotData {
kind: StackSlotKind::ExplicitSlot,
// FIXME Don't force the size to a multiple of 16 bytes once Cranelift gets a way to
// specify stack slot alignment.
let ptr = fx.create_stack_slot(
// Stack slot size may be bigger for example `[u8; 3]` which is packed into an `i32`.
// It may also be smaller for example when the type is a wrapper around an integer with a
// larger alignment than the integer.
size: (std::cmp::max(abi_param_size, layout_size) + 15) / 16 * 16,
});
let ptr = Pointer::stack_slot(stack_slot);
std::cmp::max(abi_param_size, layout_size),
u32::try_from(layout.align.pref.bytes()).unwrap(),
);
let mut offset = 0;
let mut block_params_iter = block_params.iter().copied();
for param in abi_params {
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_codegen_cranelift/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,7 @@ pub(crate) fn clif_int_or_float_cast(
&[from],
)[0];
// FIXME(bytecodealliance/wasmtime#6104) use bitcast instead of store to get from i64x2 to i128
let stack_slot = fx.bcx.create_sized_stack_slot(StackSlotData {
kind: StackSlotKind::ExplicitSlot,
size: 16,
});
let ret_ptr = Pointer::stack_slot(stack_slot);
let ret_ptr = fx.create_stack_slot(16, 16);
ret_ptr.store(fx, ret, MemFlags::trusted());
ret_ptr.load(fx, types::I128, MemFlags::trusted())
} else {
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_codegen_cranelift/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,25 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
})
}

pub(crate) fn create_stack_slot(&mut self, size: u32, align: u32) -> Pointer {
if align <= 16 {
let stack_slot = self.bcx.create_sized_stack_slot(StackSlotData {
kind: StackSlotKind::ExplicitSlot,
// FIXME Don't force the size to a multiple of 16 bytes once Cranelift gets a way to
// specify stack slot alignment.
size: (size + 15) / 16 * 16,
});
Pointer::stack_slot(stack_slot)
} else {
// Alignment is too big to handle using the above hack. Dynamically realign a stack slot
// instead. This wastes some space for the realignment.
let base_ptr = self.create_stack_slot(size + align, 16).get_addr(self);
let misalign_offset = self.bcx.ins().urem_imm(base_ptr, i64::from(align));
let realign_offset = self.bcx.ins().irsub_imm(misalign_offset, i64::from(align));
Pointer::new(self.bcx.ins().iadd(base_ptr, realign_offset))
}
}

pub(crate) fn set_debug_loc(&mut self, source_info: mir::SourceInfo) {
if let Some(debug_context) = &mut self.cx.debug_context {
let (file, line, column) =
Expand Down
23 changes: 15 additions & 8 deletions compiler/rustc_codegen_cranelift/src/driver/aot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,12 +361,26 @@ pub(crate) fn run_aot(
metadata: EncodedMetadata,
need_metadata_module: bool,
) -> Box<OngoingCodegen> {
// FIXME handle `-Ctarget-cpu=native`
let target_cpu = match tcx.sess.opts.cg.target_cpu {
Some(ref name) => name,
None => tcx.sess.target.cpu.as_ref(),
}
.to_owned();

let cgus = if tcx.sess.opts.output_types.should_codegen() {
tcx.collect_and_partition_mono_items(()).1
} else {
// If only `--emit metadata` is used, we shouldn't perform any codegen.
// Also `tcx.collect_and_partition_mono_items` may panic in that case.
&[]
return Box::new(OngoingCodegen {
modules: vec![],
allocator_module: None,
metadata_module: None,
metadata,
crate_info: CrateInfo::new(tcx, target_cpu),
concurrency_limiter: ConcurrencyLimiter::new(tcx.sess, 0),
});
};

if tcx.dep_graph.is_fully_enabled() {
Expand Down Expand Up @@ -481,13 +495,6 @@ pub(crate) fn run_aot(
None
};

// FIXME handle `-Ctarget-cpu=native`
let target_cpu = match tcx.sess.opts.cg.target_cpu {
Some(ref name) => name,
None => tcx.sess.target.cpu.as_ref(),
}
.to_owned();

Box::new(OngoingCodegen {
modules,
allocator_module,
Expand Down
Loading

0 comments on commit 642bfb2

Please sign in to comment.