| Age | Commit message (Collapse) | Author | Lines |
|---|
|
|
|
The conversation in 143502 made be realize how easy this is to handle, since the only possibilty is ZSTs -- everything else ends up with the destination being `LocalKind::Memory` and thus doesn't call `codegen_rvalue_operand` at all.
This gets us perilously close to a world where `rvalue_creates_operand` only ever returns true. I'll try out such a world next :)
|
|
Simplify discriminant codegen for niche-encoded variants which don't wrap across an integer boundary
Inspired by rust-lang/rust#139729, this attempts to be a much-simpler and more-localized change while still making a difference. (Specifically, this does not try to solve the problem with select-sinking, leaving that to be fixed by https://github.com/llvm/llvm-project/issues/134024 -- once it gets released -- instead of in rustc's codegen.)
What this *does* improve is checking for the variant in a 3+ variant enum when that variant is the type providing the niche. Something like `if let Foo::WithBool(_) = ...` previously compiled to `ugt(add(x, -2), 2)`, which is non-trivial to think about because it's depending on the unsigned wrapping to shift the 0/1 up above 2. With this PR it compiles to just `ult(x, 2)`, which is probably what you'd have written yourself if you were doing it by hand to look for "is this byte a bool?".
That's done by leaving most of the codegen alone, but adding a couple new special cases to the `is_niche` check. The default looks at the relative discriminant, but in the common cases where there's no wraparound involved, we can just check the original value, rather than the offsetted one.
The first commit just adds some tests, so the best way to see the effect of this change is to look at the second commit and how it updates the test expectations.
|
|
|
|
and use it for naked functions
|
|
|
|
|
|
Rollup of 8 pull requests
Successful merges:
- rust-lang/rust#142391 (rust: library: Add `setsid` method to `CommandExt` trait)
- rust-lang/rust#143302 (`tests/ui`: A New Order [27/N])
- rust-lang/rust#143303 (`tests/ui`: A New Order [28/28] FINAL PART)
- rust-lang/rust#143568 (std: sys: net: uefi: tcp4: Add timeout support)
- rust-lang/rust#143611 (Mention more APIs in `ParseIntError` docs)
- rust-lang/rust#143661 (chore: Improve how the other suggestions message gets rendered)
- rust-lang/rust#143708 (fix: Include frontmatter in -Zunpretty output )
- rust-lang/rust#143718 (Make UB transmutes really UB in LLVM)
r? `@ghost`
`@rustbot` modify labels: rollup
try-job: i686-gnu-nopt-1
try-job: test-various
|
|
Make UB transmutes really UB in LLVM
Ralf suggested in <https://github.com/rust-lang/rust/pull/143410#discussion_r2184928123> that UB transmutes shouldn't be trapping, which happened for the one path *that* PR was changing, but there's another path as well, so *this* PR changes that other path to match.
r? codegen
|
|
Remove support for dynamic allocas
Followup to rust-lang/rust#141811
|
|
So places that need `unreachable` but in the middle of a basic block can call that instead of figuring out the best way to do it.
|
|
Add opaque TypeId handles for CTFE
Reopen of https://github.com/rust-lang/rust/pull/142789#issuecomment-3053155043 after some bors insta-merge chaos
r? `@RalfJung`
|
|
Ralf suggested in <https://github.com/rust-lang/rust/pull/143410#discussion_r2184928123> that UB transmutes shouldn't be trapping, which happened for the one path that PR was changing, but there's another path as well, so this PR changes that other path to match.
|
|
Let `rvalue_creates_operand` return true for *all* `Rvalue::Aggregate`s
~~Draft for now because it's built on Ralf's rust-lang/rust#143291~~
Inspired by https://github.com/rust-lang/rust/pull/138759#discussion_r2156375342 where I noticed that we were nearly at this point, plus the comments I was writing in rust-lang/rust#143410 that reminded me a type-dependent `true` is fine.
This PR splits the `OperandRef::builder` logic out to a separate type, with the updates needed to handle SIMD as well. In doing so, that makes the existing `Aggregate` path in `codegen_rvalue_operand` capable of handing SIMD values just fine.
As a result, we no longer need to do layout calculations for aggregate result types when running the analysis to determine which things can be SSA in codegen.
|
|
|
|
|
|
Inspired by <https://github.com/rust-lang/rust/pull/138759#discussion_r2156375342> where I noticed that we were nearly at this point, plus the comments I was writing in 143410 that reminded me a type-dependent `true` is fine.
This PR splits the `OperandRef::builder` logic out to a separate type, with the updates needed to handle SIMD as well. In doing so, that makes the existing `Aggregate` path in `codegen_rvalue_operand` capable of handing SIMD values just fine.
As a result, we no longer need to do layout calculations for aggregate result types when running the analysis to determine which things can be SSA in codegen.
|
|
|
|
Allow custom default address spaces and parse `p-` specifications in the datalayout string
Some targets, such as CHERI, use as default an address space different from the "normal" default address space `0` (in the case of CHERI, [200 is used](https://www.cl.cam.ac.uk/techreports/UCAM-CL-TR-877.pdf)). Currently, `rustc` does not allow to specify custom address spaces and does not take into consideration [`p-` specifications in the datalayout string](https://llvm.org/docs/LangRef.html#langref-datalayout).
This patch tries to mitigate these problems by allowing targets to define a custom default address space (while keeping the default value to address space `0`) and adding the code to parse the `p-` specifications in `rustc_abi`. The main changes are that `TargetDataLayout` now uses functions to refer to pointer-related informations, instead of having specific fields for the size and alignment of pointers in the default address space; furthermore, the two `pointer_size` and `pointer_align` fields in `TargetDataLayout` are replaced with an `FxHashMap` that holds info for all the possible address spaces, as parsed by the `p-` specifications.
The potential performance drawbacks of not having ad-hoc fields for the default address space will be tested in this PR's CI run.
r? workingjubilee
|
|
default data address space
|
|
|
|
r=RalfJung,workingjubilee
Block SIMD in transmute_immediate; delete `OperandValueKind`
Vectors have been causing me problems for years in this code, for example https://github.com/rust-lang/rust/pull/110021#discussion_r1160975086 and https://github.com/rust-lang/rust/pull/143194
See conversation in <https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/near/526262799>.
By blocking SIMD in `transmute_immediate` it can be simplified to just take the `Scalar`s involved -- the backend types can be gotten from those `Scalar`s, rather than needing to be passed. And there's an assert added to ICE it if it does get hit.
Accordingly, this changes `rvalue_creates_operand` to not send SIMD transmutes through the operand path, but to always go through memory instead, like they did back before rust-lang/rust#108442.
And thanks to those changes, I could also remove the `OperandValueKind` type that I added back then which `@RalfJung` rightly considers pretty sketchy.
cc `@folkertdev` `@workingjubilee` from the zulip conversation too
|
|
|
|
|
|
|
|
See conversation in <https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/near/526262799>.
|
|
r=workingjubilee
fix bitcast of single-element SIMD vectors
in effect this reverts https://github.com/rust-lang/rust/pull/142768 and adds additional tests. That PR relaxed the conditions on an early return in an incorrect way that would create broken LLVM IR.
https://godbolt.org/z/PaaGWTv5a
```rust
#![feature(repr_simd)]
#[repr(simd)]
#[derive(Clone, Copy)]
struct S([i64; 1]);
#[no_mangle]
pub extern "C" fn single_element_simd(b: S) -> i64 {
unsafe { std::mem::transmute(b) }
}
```
at the time of writing generates this LLVM IR, where the type of the return is different from the function's return type.
```llvm
define noundef i64 ``````@single_element_simd(<1`````` x i64> %b) unnamed_addr {
start:
ret <1 x i64> %b
}
```
The test output is actually the same for the existing tests, showing that the change didn't actually matter for any tested behavior. It is probably a bit faster to do the early return, but, well, it's incorrect in general.
zullip thread: [#t-compiler > Is transmuting a `T` to `Tx1` (one-element SIMD vector) UB?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Is.20transmuting.20a.20.60T.60.20to.20.60Tx1.60.20.28one-element.20SIMD.20vector.29.20UB.3F/with/526262799)
cc ``````@sayantn``````
r? ``````@scottmcm``````
|
|
|
|
Rollup of 14 pull requests
Successful merges:
- rust-lang/rust#142429 (`tests/ui`: A New Order [13/N])
- rust-lang/rust#142514 (Miri: handling of SNaN inputs in `f*::pow` operations)
- rust-lang/rust#143066 (Use let chains in the new solver)
- rust-lang/rust#143090 (Workaround for memory unsafety in third party DLLs)
- rust-lang/rust#143118 (`tests/ui`: A New Order [15/N])
- rust-lang/rust#143159 (Do not freshen `ReError`)
- rust-lang/rust#143168 (`tests/ui`: A New Order [16/N])
- rust-lang/rust#143176 (fix typos and improve clarity in documentation)
- rust-lang/rust#143187 (Add my work email to mailmap)
- rust-lang/rust#143190 (Use the `new` method for `BasicBlockData` and `Statement`)
- rust-lang/rust#143195 (`tests/ui`: A New Order [17/N])
- rust-lang/rust#143196 (Port #[link_section] to the new attribute parsing infrastructure)
- rust-lang/rust#143199 (Re-disable `tests/run-make/short-ice` on Windows MSVC again)
- rust-lang/rust#143219 (Show auto trait and blanket impls for `!`)
r? `@ghost`
`@rustbot` modify labels: rollup
|
|
|
|
deciding not to
|
|
|
|
Avoid introducing a large number of changes when adding optional initialization fields.
|
|
Improve documentation of `TagEncoding`
This PR is follow-up from the [discussion here](https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/.E2.9C.94.20VariantId.3DDiscriminant.20when.20tag.20is.20niche.20encoded.3F/with/524384295).
It aims at making the `TagEncoding` documentation less ambiguous and more detailed with references to relevant implementation sides. It especially clears up the ambiguous use of discriminant/variant index, which sparked the discussion referenced above.
PS: While working with layout data, I somehow ended up looking at the docs for `FakeBorrowKind` and noticed that the one example was not in a doc comment. I hope that this is minor enough of a fix for it to be okay in this otherwise unrelated PR.
|
|
|
|
Insert checks for enum discriminants when debug assertions are enabled
Similar to the existing null-pointer and alignment checks, this checks for valid enum discriminants on creation of enums through unsafe transmutes. Essentially this sanitizes patterns like the following:
```rust
let val: MyEnum = unsafe { std::mem::transmute<u32, MyEnum>(42) };
```
An extension of this check will be done in a follow-up that explicitly sanitizes for extern enum values that come into Rust from e.g. C/C++.
This check is similar to Miri's capabilities of checking for valid construction of enum values.
This PR is inspired by saethlin@'s PR
https://github.com/rust-lang/rust/pull/104862. Thank you so much for keeping this code up and the detailed comments!
I also pair-programmed large parts of this together with vabr-g@.
r? `@saethlin`
|
|
Similar to the existing nullpointer and alignment checks, this checks
for valid enum discriminants on creation of enums through unsafe
transmutes. Essentially this sanitizes patterns like the following:
```rust
let val: MyEnum = unsafe { std::mem::transmute<u32, MyEnum>(42) };
```
An extension of this check will be done in a follow-up that explicitly
sanitizes for extern enum values that come into Rust from e.g. C/C++.
This check is similar to Miri's capabilities of checking for valid
construction of enum values.
This PR is inspired by saethlin@'s PR
https://github.com/rust-lang/rust/pull/104862. Thank you so much for
keeping this code up and the detailed comments!
I also pair-programmed large parts of this together with vabr-g@.
|
|
Avoid a bitcast FFI call in transmuting
For things that only change the valid ranges, we can just return the input, rather than making the `LLVMBuildBitCast` call and having *it* then do nothing.
I tried to tweak this a bit more and broke stuff, so I also added some extra tests for that as we apparently didn't have coverage.
|
|
|
|
For things that only change the valid ranges, we can just skip the `LLVMBuildBitCast` call.
I tried to tweak this a bit more and broke stuff, so I also added some extra tests for that as we apparently didn't have coverage.
|
|
apply clippy::or_fun_call
Applies https://rust-lang.github.io/rust-clippy/master/index.html?groups=nursery#or_fun_call to reduce needless allocs.
|
|
CodeGen: rework Aggregate implemention for rvalue_creates_operand cases
A non-trivial refactor pulled out from rust-lang/rust#138759
r? workingjubilee
The previous implementation I'd written here based on `index_by_increasing_offset` is complicated to follow and difficult to extend to non-structs.
This changes the implementation, without actually changing any codegen (thus no test changes either), to be more like the existing `extract_field` (<https://github.com/rust-lang/rust/blob/2b0274c71dba0e24370ebf65593da450e2e91868/compiler/rustc_codegen_ssa/src/mir/operand.rs#L345-L425>) in that it allows setting a particular field directly.
Notably I've found this one much easier to get right, in particular because having the `OperandRef<Result<V, Scalar>>` gives a really useful thing to include in ICE messages if something did happen to go wrong.
|
|
Another refactor pulled out from 138759
The previous implementation I'd written here based on `index_by_increasing_offset` is complicated to follow and difficult to extend to non-structs.
This changes the implementation, without actually changing any codegen (thus no test changes either), to be more like the existing `extract_field` (<https://github.com/rust-lang/rust/blob/2b0274c71dba0e24370ebf65593da450e2e91868/compiler/rustc_codegen_ssa/src/mir/operand.rs#L345-L425>) in that it allows setting a particular field directly.
Notably I've found this one much easier to get right, in particular because having the `OperandRef<Result<V, Scalar>>` gives a really useful thing to include in ICE messages if something did happen to go wrong.
|
|
|
|
|
|
|
|
|
|
No changes; just removing the `self` that wasn't needed.
|
|
In PR 90877 T-lang decided not to remove `intrinsics::pref_align_of`.
However, the intrinsic and its supporting code
1. is a nightly feature, so can be removed at compiler/libs discretion
2. requires considerable effort in the compiler to support, as it
necessarily complicates every single site reasoning about alignment
3. has been justified based on relevance to codegen, but it is only a
requirement for C++ (not C, not Rust) stack frame layout for AIX,
in ways Rust would not consider even with increased C++ interop
4. is only used by rustc to overalign some globals, not correctness
5. can be adequately replaced by other rules for globals, as it mostly
affects alignments for a few types under 16 bytes of alignment
6. has only one clear benefactor: automating C -> Rust translation
for GNU extensions like `__alignof`
7. such code was likely intended to be `alignof` or `_Alignof`,
because the GNU extension is a "false friend" of the C keyword,
which makes the choice to support such a mapping very questionable
8. makes it easy to do incorrect codegen in the compiler by its mere
presence as usual Rust rules of alignment (e.g. `size == align * N`)
do not hold with preferred alignment
The implementation is clearly damaging the code quality of the compiler.
Thus it is within the compiler team's purview to simply rip it out.
If T-lang wishes to have this intrinsic restored for c2rust's benefit,
it would have to use a radically different implementation that somehow
does not cause internal incorrectness.
Until then, remove the intrinsic and its supporting code, as one tool
and an ill-considered GCC extension cannot justify risking correctness.
Because we touch a fair amount of the compiler to change this at all,
and unfortunately the duplication of AbiAndPrefAlign is deep-rooted,
we keep an "AbiAlign" type which we can wean code off later.
|
|
|
