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

Don't include global asm in mir_keys, fix error body synthesis #137502

Merged
merged 3 commits into from
Mar 9, 2025
Merged
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
3 changes: 2 additions & 1 deletion compiler/rustc_mir_build/src/builder/mod.rs
Original file line number Diff line number Diff line change
@@ -612,7 +612,8 @@ fn construct_error(tcx: TyCtxt<'_>, def_id: LocalDefId, guar: ErrorGuaranteed) -
| DefKind::AssocConst
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::Static { .. } => (vec![], tcx.type_of(def_id).instantiate_identity(), None),
| DefKind::Static { .. }
| DefKind::GlobalAsm => (vec![], tcx.type_of(def_id).instantiate_identity(), None),
DefKind::Ctor(..) | DefKind::Fn | DefKind::AssocFn => {
let sig = tcx.liberate_late_bound_regions(
def_id.to_def_id(),
4 changes: 4 additions & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
@@ -316,6 +316,10 @@ fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet<LocalDefId> {
// All body-owners have MIR associated with them.
let mut set: FxIndexSet<_> = tcx.hir_body_owners().collect();

// Remove the fake bodies for `global_asm!`, since they're not useful
// to be emitted (`--emit=mir`) or encoded (in metadata).
set.retain(|&def_id| !matches!(tcx.def_kind(def_id), DefKind::GlobalAsm));

// Coroutine-closures (e.g. async closures) have an additional by-move MIR
// body that isn't in the HIR.
for body_owner in tcx.hir_body_owners() {
8 changes: 2 additions & 6 deletions tests/codegen/target-feature-inline-closure.rs
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ use std::arch::x86_64::*;
#[cfg(target_arch = "x86_64")]
#[target_feature(enable = "avx")]
fn with_avx(x: __m256) -> __m256 {
// CHECK: fadd
// CHECK: fadd <8 x float>
let add = {
#[inline(always)]
|x, y| unsafe { _mm256_add_ps(x, y) }
@@ -24,14 +24,10 @@ fn with_avx(x: __m256) -> __m256 {
#[no_mangle]
#[cfg(target_arch = "x86_64")]
unsafe fn without_avx(x: __m256) -> __m256 {
// CHECK-NOT: fadd
// CHECK-NOT: fadd <8 x float>
let add = {
#[inline(always)]
|x, y| unsafe { _mm256_add_ps(x, y) }
};
add(x, x)
}

// Don't allow the above CHECK-NOT to accidentally match a commit hash in the
// compiler version.
// CHECK-LABEL: rustc version
26 changes: 26 additions & 0 deletions tests/ui/asm/global-asm-isnt-really-a-mir-body.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//@ revisions: emit_mir instrument cfi

// Make sure we don't try to emit MIR for it.
//@[emit_mir] compile-flags: --emit=mir

// Make sure we don't try to instrument it.
//@[instrument] compile-flags: -Cinstrument-coverage -Zno-profiler-runtime
//@[instrument] only-linux

// Make sure we don't try to CFI encode it.
//@[cfi] compile-flags: -Zsanitizer=cfi -Ccodegen-units=1 -Clto -Ctarget-feature=-crt-static -Clink-dead-code=true
//@[cfi] needs-sanitizer-cfi
//@[cfi] no-prefer-dynamic
// FIXME(#122848) Remove only-linux once OSX CFI binaries work
//@[cfi] only-linux

//@ build-pass
//@ needs-asm-support

use std::arch::global_asm;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't stable for all architectures 🙃

Copy link
Member Author

@compiler-errors compiler-errors Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, is there a needs-global-asm 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or maybe needs-asm

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I... thiiink so?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs-asm-support


fn foo() {}

global_asm!("/* {} */", sym foo);

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/asm/global-asm-with-error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Ensure that we don't ICE when constructing the fake MIR body for a global
// asm when the body has errors. See #137470.

//@ needs-asm-support

use std::arch::global_asm;

global_asm!("/* {} */", sym a);
//~^ ERROR cannot find value `a` in this scope

fn main() {}
9 changes: 9 additions & 0 deletions tests/ui/asm/global-asm-with-error.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0425]: cannot find value `a` in this scope
--> $DIR/global-asm-with-error.rs:8:29
|
LL | global_asm!("/* {} */", sym a);
| ^ not found in this scope

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0425`.
Loading