diff --git a/CHANGELOG.md b/CHANGELOG.md index 21845ecd26..c6b468914f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,9 +33,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [PR#1011](https://github.com/EmbarkStudios/rust-gpu/pull/1011) made `NonWritable` all read-only storage buffers (i.e. those typed `&T`, where `T` doesn't have interior mutability) ### Fixed 🩹 +- [PR#1025](https://github.com/EmbarkStudios/rust-gpu/pull/1025) fixed [#1024](https://github.com/EmbarkStudios/rust-gpu/issues/1024) by keeping checked arithmetic "zombie" `bool`s disjoint from normal `bool` (`false`) consts - [PR#1023](https://github.com/EmbarkStudios/rust-gpu/pull/1023) fixed [#1021](https://github.com/EmbarkStudios/rust-gpu/issues/1021) by always inlining calls with "not obviously legal" pointer args (instead of only inlining calls with "obviously illegal" pointer args) - [PR#1009](https://github.com/EmbarkStudios/rust-gpu/pull/1009) fixed [#1008](https://github.com/EmbarkStudios/rust-gpu/issues/1008) by reinstating mutability checks for entry-point parameters pointing into read-only storage classes (e.g. `#[spirv(uniform)] x: &mut u32` is now again an error) -- [PR#995](https://github.com/EmbarkStudios/rust-gpu/pull/995) fixed [#994](https://github.com/EmbarkStudios/rust-gpu/issues/994) by using `OpAtomicFAddEXT` instead of `OpAtomicFMaxEXT` in `atomic_f_add`. +- [PR#995](https://github.com/EmbarkStudios/rust-gpu/pull/995) fixed [#994](https://github.com/EmbarkStudios/rust-gpu/issues/994) by using `OpAtomicFAddEXT` instead of `OpAtomicFMaxEXT` in `atomic_f_add` ## [0.6.1] @@ -45,13 +46,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [0.6.0] ### Added ⭐ -- [PR#998](https://github.com/EmbarkStudios/rust-gpu/pull/998) added `extra_arg()` SpirvBuilder API to be able to set codegen args otherwise not supported by the API (for example, to set `--spirv-passes`). +- [PR#998](https://github.com/EmbarkStudios/rust-gpu/pull/998) added `extra_arg()` SpirvBuilder API to be able to set codegen args otherwise not supported by the API (for example, to set `--spirv-passes`) ### Changed 🛠 -- [PR#999](https://github.com/EmbarkStudios/rust-gpu/pull/999) Made the [`SPIR-🇹` shader IR framework](https://github.com/EmbarkStudios/spirt) the default. You can opt-out using `--no-spirt` codegen arg. -- [PR#992](https://github.com/EmbarkStudios/rust-gpu/pull/992) renamed `rust-toolchain` to `rust-toolchain.toml`. -- [PR#991](https://github.com/EmbarkStudios/rust-gpu/pull/991) updated toolchain to `nightly-2023-01-21`. -- [PR#990](https://github.com/EmbarkStudios/rust-gpu/pull/990) removed return type inference from `Image` API and made `glam` usage mandatory. +- [PR#999](https://github.com/EmbarkStudios/rust-gpu/pull/999) made the [`SPIR-🇹` shader IR framework](https://github.com/EmbarkStudios/spirt) the default (you can opt out via `RUSTGPU_CODEGEN_ARGS=--no-spirt`) +- [PR#992](https://github.com/EmbarkStudios/rust-gpu/pull/992) renamed `rust-toolchain` to `rust-toolchain.toml` +- [PR#991](https://github.com/EmbarkStudios/rust-gpu/pull/991) updated toolchain to `nightly-2023-01-21` +- [PR#990](https://github.com/EmbarkStudios/rust-gpu/pull/990) removed return type inference from `Image` API and made `glam` usage mandatory ## [0.5.0] diff --git a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs index a4fe63db38..312b6f5366 100644 --- a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs +++ b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs @@ -933,11 +933,14 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { lhs: Self::Value, rhs: Self::Value, ) -> (Self::Value, Self::Value) { - let fals = self.constant_bool(self.span(), false); + // NOTE(eddyb) this needs to be `undef`, not `false`/`true`, because + // we don't want the user's boolean constants to keep the zombie alive. + let bool = SpirvType::Bool.def(self.span(), self); + let overflowed = self.undef(bool); let result = match oop { - OverflowOp::Add => (self.add(lhs, rhs), fals), - OverflowOp::Sub => (self.sub(lhs, rhs), fals), - OverflowOp::Mul => (self.mul(lhs, rhs), fals), + OverflowOp::Add => (self.add(lhs, rhs), overflowed), + OverflowOp::Sub => (self.sub(lhs, rhs), overflowed), + OverflowOp::Mul => (self.mul(lhs, rhs), overflowed), }; self.zombie( result.1.def(self), diff --git a/tests/ui/lang/consts/issue-1024.rs b/tests/ui/lang/consts/issue-1024.rs new file mode 100644 index 0000000000..a02057f0b5 --- /dev/null +++ b/tests/ui/lang/consts/issue-1024.rs @@ -0,0 +1,31 @@ +// Tests that the zombie `bool` from `overflowing_*` (the "has overflowed" value) +// isn't kept alive by the user's own (unrelated) `bool` constants. +// +// NOTE(eddyb) quick explanation for the compile-flags below: +// * `-Coverflow-checks=off`: the default, but explicit here because of how +// its behavior is relied upon by `u32_overflowing_add_only_result` +// * `-Ccodegen-units=1`: avoids `bool` consts and `::add` being +// in separate CGUs (which then bypasses const deduplication, during linking) + +// build-pass +// compile-flags: -Coverflow-checks=off -Ccodegen-units=1 + +use spirv_std::spirv; + +const PRIVATE_GLOBAL_BOOLS: &[bool; 2] = &[false, true]; + +/// `a.overflowing_add(b).0` equivalent, but with the Rust `(u32, bool`) never +/// existing, and the `.0` happening in the compiler, via `CheckedBinaryOp(Add)` +/// in `::add`, and `-Coverflow-checks=off` ignoring the `.1` `bool`. +fn u32_overflowing_add_only_result(a: u32, b: u32) -> u32 { + ::add(a, b) +} + +#[spirv(fragment)] +pub fn main(x: &mut u32) { + *x = 0; + for i in 0..PRIVATE_GLOBAL_BOOLS.len() { + let y = (PRIVATE_GLOBAL_BOOLS[i] as u32) << i; + *x = u32_overflowing_add_only_result(*x, y); + } +}