Revert "Use the same DISubprogram for each instance of the same inline function within the caller"
This reverts commit 687bffa493.
Reverting to resolve ICEs reported on nightly.
cc `@dpaoliello`
Fixes#115156
Fix#115150 by encoding f32 and f64 correctly for cross-language CFI. I
missed changing the encoding for f32 and f64 when I introduced the
integer normalization option in #105452 as integer normalization does
not include floating point. `f32` and `f64` should be always encoded as
`f` and `d` since they are both FFI safe when their representation are
the same (i.e., IEEE 754) for both the Rust compiler and Clang.
Add regression test for not `memcpy`ing padding bytes
Closes#56297
See this comparison: https://rust.godbolt.org/z/jjzfonfcE
I don't have any experience with codegen tests, I hope this is correct
Use the same DISubprogram for each instance of the same inlined function within a caller
# Issue Details:
The call to `panic` within a function like `Option::unwrap` is translated to LLVM as a `tail call` (as it will never return), when multiple calls to the same function like this is inlined LLVM will notice the common `tail call` block (i.e., loading the same panic string + location info and then calling `panic`) and merge them together.
When merging these instructions together, LLVM will also attempt to merge the debug locations as well, but this fails (i.e., debug info is dropped) as Rust emits a new `DISubprogram` at each inline site thus LLVM doesn't recognize that these are actually the same function and so thinks that there isn't a common debug location.
As an example of this when building for x86_64 Windows (note the lack of `.cv_loc` before the call to `panic`, thus it will be attributed to the same line at the `addq` instruction):
```
.cv_loc 0 1 23 0 # src\lib.rs:23:0
addq $40, %rsp
retq
leaq .Lalloc_f570dea0a53168780ce9a91e67646421(%rip), %rcx
leaq .Lalloc_629ace53b7e5b76aaa810d549cc84ea3(%rip), %r8
movl $43, %edx
callq _ZN4core9panicking5panic17h12e60b9063f6dee8E
int3
```
# Fix Details:
Cache the `DISubprogram` emitted for each inlined function instance within a caller so that this can be reused if that instance is encountered again, this also requires caching the `DILexicalBlock` and `DIVariable` objects to avoid creating duplicates.
After this change the above assembly now looks like:
```
.cv_loc 0 1 23 0 # src\lib.rs:23:0
addq $40, %rsp
retq
.cv_inline_site_id 5 within 0 inlined_at 1 0 0
.cv_inline_site_id 6 within 5 inlined_at 1 12 0
.cv_loc 6 2 935 0 # library\core\src\option.rs:935:0
leaq .Lalloc_5f55955de67e57c79064b537689facea(%rip), %rcx
leaq .Lalloc_e741d4de8cb5801e1fd7a6c6795c1559(%rip), %r8
movl $43, %edx
callq _ZN4core9panicking5panic17hde1558f32d5b1c04E
int3
```
We've investigated one reason why debugging information often goes wrong at https://reviews.llvm.org/D152095.
> LLVM can't handle IR where subprogram definitions are nested within DICompositeType when doing LTO builds,
> because there's no good way to cross the CU boundary to insert a nested DISubprogram definition in one CU into a type defined in another CU.
Add a new `compare_bytes` intrinsic instead of calling `memcmp` directly
As discussed in #113435, this lets the backends be the place that can have the "don't call the function if n == 0" logic, if it's needed for the target. (I didn't actually *add* those checks, though, since as I understood it we didn't actually need them on known targets?)
Doing this also let me make it `const` (unstable), which I don't think `extern "C" fn memcmp` can be.
cc `@RalfJung` `@Amanieu`
Nest other codegen test topics
This PR is like rust-lang/rust#114229 in that it mostly pushes codegen tests around, shoving them into their own directories, but because all of the changes are very simple cleanups I pulled them into a separate PR. The other PR might involve actually evaluating the correctness of the test after changes, but here it is mostly a matter of taste. The only "functional" change is deleting a few tests that... hinge on a version of LLVM that we don't support (as of rust-lang/rust#114148 anyways).
I considered a few different ways to group other topics but I feel the question of whether `tests/codegen/{vec,array,slice}` should exist is more subtle than these choices, as it might be better to group such related tests by other topics like bounds check elision, thus I avoided making it.
Nest tests/codegen/sanitizer*.rs tests in sanitizer dir
The sanitizer tests are the largest and most meticulously tested set of tests in tests/codegen. That's good! They all clearly belong to a subject and thus could go in a directory, but are not, instead being placed simply in tests/codegen. That's bad! Fix this by placing them in their own directory and renaming them to be less repetitive after that move.
A few tests are brittle, and embed their filename in the test's checks. This is acceptable for the ones where it is used only two times, but one test embeds the test's mangled filename in the test *over 50 times*! This may have been one of the things discouraging anyone from moving it, and thus from moving the set. Fortunately, I have some knowledge of Itanium mangling (involuntarily), regex, and the FileCheck syntax. With a capturing variable, FileCheck allows us to now move this test around again without diffing it on ~50 lines, while still guaranteeing that the mangled substring is the same each time.
This also clarifies why the substring is repeated a zillion times, instead of being cryptic. They don't call it mangling because the result is pretty and easy to understand, but now it is slightly easier! Yay descriptive variables!
cleanup: remove pointee types
This can't be merged until the oldest LLVM version we support uses opaque pointers, which will be the case after #114148. (Also note `-Cllvm-args="-opaque-pointers=0"` can technically be used in LLVM 15, though I don't think we should support that configuration.)
I initially hoped this would provide some minor perf win, but in https://github.com/rust-lang/rust/pull/105412#issuecomment-1341224450 it had very little impact, so this is only valuable as a cleanup.
As a followup, this will enable #96242 to be resolved.
r? `@ghost`
`@rustbot` label S-blocked
In the basic case, simply do the string substitution.
For one case with many instances, capture the Itanium-
mangled filename and assert its reuse instead.
Combining revisions with only-arch allows specifying
that a test only applies to a handful of targets.
This allows removing a large amount of repetition
in the test suite for tests that do not benefit.
The revisions are suboptimal for this for some tests,
so they aren't preferred in those cases.
Slightly more complicated: also give them appropriate names
that somewhat describe the cases they are trying to cover,
using information from PR chatter in rust-lang/rust#47158
Fix #[inline(always)] on closures with target feature 1.1
Fixes#108655. I think this is the most obvious solution that isn't overly complicated. The comment includes more justification, but I think this is likely better than demoting the `#[inline(always)]` to `#[inline]`, since existing code is unaffected.
Both GCC and Clang write by default a `.comment` section with compiler
information:
```txt
$ gcc -c -xc /dev/null && readelf -p '.comment' null.o
String dump of section '.comment':
[ 1] GCC: (GNU) 11.2.0
$ clang -c -xc /dev/null && readelf -p '.comment' null.o
String dump of section '.comment':
[ 1] clang version 14.0.1 (https://github.com/llvm/llvm-project.git c62053979489ccb002efe411c3af059addcb5d7d)
```
They also implement the `-Qn` flag to avoid doing so:
```txt
$ gcc -Qn -c -xc /dev/null && readelf -p '.comment' null.o
readelf: Warning: Section '.comment' was not dumped because it does not exist!
$ clang -Qn -c -xc /dev/null && readelf -p '.comment' null.o
readelf: Warning: Section '.comment' was not dumped because it does not exist!
```
So far, `rustc` only does it for WebAssembly targets and only
when debug info is enabled:
```txt
$ echo 'fn main(){}' | rustc --target=wasm32-unknown-unknown --emit=llvm-ir -Cdebuginfo=2 - && grep llvm.ident rust_out.ll
!llvm.ident = !{!27}
```
In the RFC part of this PR it was decided to always add
the information, which gets us closer to other popular compilers.
An opt-out flag like GCC and Clang may be added later on if deemed
necessary.
Implementation-wise, this covers both `ModuleLlvm::new()` and
`ModuleLlvm::new_metadata()` cases by moving the addition to
`context::create_module` and adds a few test cases.
ThinLTO also sees the `llvm.ident` named metadata duplicated (in
temporary outputs), so this deduplicates it like it is done for
`wasm.custom_sections`. The tests also check this duplication does
not take place.
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Turn copy into moves during DSE.
Dead store elimination computes whether removing a direct store to an unborrowed place is allowed.
Where removing a store is allowed, writing `uninit` is too.
This means that we can use this pass to transform `copy` operands into `move` operands. This is only interesting in call terminators, so we only handle those.
Special care is taken for the `use_both(_1, _1)` case:
- moving the second argument is ok, as `_1` is not live after the call;
- moving the first argument is not, as the second argument reads `_1`.
Fixes#75993
Fixes https://github.com/rust-lang/rust/issues/108068
r? `@RalfJung`
cc `@JakobDegen`
Resurrect: rustc_target: Add alignment to indirectly-passed by-value types, correcting the alignment of byval on x86 in the process.
Same as #111551, which I [accidentally closed](https://github.com/rust-lang/rust/pull/111551#issuecomment-1571222612) :/
---
This resurrects PR #103830, which has sat idle for a while.
Beyond #103830, this also:
- fixes byval alignment for types containing vectors on Darwin (see `tests/codegen/align-byval-vector.rs`)
- fixes byval alignment for overaligned types on x86 Windows (see `tests/codegen/align-byval.rs`)
- fixes ABI for types with 128bit requested alignment on ARM64 Linux (see `tests/codegen/aarch64-struct-align-128.rs`)
r? `@nikic`
---
`@pcwalton's` original PR description is reproduced below:
Commit 88e4d2c from five years ago removed
support for alignment on indirectly-passed arguments because of problems with
the `i686-pc-windows-msvc` target. Unfortunately, the `memcpy` optimizations I
recently added to LLVM 16 depend on this to forward `memcpy`s. This commit
attempts to fix the problems with `byval` parameters on that target and now
correctly adds the `align` attribute.
The problem is summarized in [this comment] by `@eddyb.` Briefly, 32-bit x86 has
special alignment rules for `byval` parameters: for the most part, their
alignment is forced to 4. This is not well-documented anywhere but in the Clang
source. I looked at the logic in Clang `TargetInfo.cpp` and tried to replicate
it here. The relevant methods in that file are
`X86_32ABIInfo::getIndirectResult()` and
`X86_32ABIInfo::getTypeStackAlignInBytes()`. The `align` parameter attribute
for `byval` parameters in LLVM must match the platform ABI, or miscompilations
will occur. Note that this doesn't use the approach suggested by eddyb, because
I felt it was overkill to store the alignment in `on_stack` when special
handling is really only needed for 32-bit x86.
As a side effect, this should fix#80127, because it will make the `align`
parameter attribute for `byval` parameters match the platform ABI on LLVM
x86-64.
[this comment]: #80822 (comment)
