Skip to content

fix handling of ZST in win64 ABI on windows-msvc targets #135204

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

Merged
merged 2 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
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
4 changes: 4 additions & 0 deletions compiler/rustc_abi/src/extern_abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ pub fn is_enabled(
s
}

/// Returns whether the ABI is stable to use.
///
/// Note that there is a separate check determining whether the ABI is even supported
/// on the current target; see `fn is_abi_supported` in `rustc_target::spec`.
pub fn is_stable(name: &str) -> Result<(), AbiDisabled> {
match name {
// Stable
Expand Down
29 changes: 14 additions & 15 deletions compiler/rustc_target/src/callconv/mod.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
use std::str::FromStr;
use std::{fmt, iter};

pub use rustc_abi::{Reg, RegKind};
pub use rustc_abi::{ExternAbi, Reg, RegKind};
use rustc_macros::HashStable_Generic;
use rustc_span::Symbol;

use crate::abi::{
self, AddressSpace, Align, BackendRepr, HasDataLayout, Pointer, Size, TyAbiInterface,
TyAndLayout,
};
use crate::spec::abi::Abi as SpecAbi;
use crate::spec::{self, HasTargetSpec, HasWasmCAbiOpt, HasX86AbiOpt, WasmCAbi};
use crate::spec::{HasTargetSpec, HasWasmCAbiOpt, HasX86AbiOpt, WasmCAbi};

mod aarch64;
mod amdgpu;
Expand Down Expand Up @@ -627,20 +626,20 @@ impl<'a, Ty: fmt::Display> fmt::Debug for FnAbi<'a, Ty> {
#[derive(Copy, Clone, Debug, HashStable_Generic)]
pub enum AdjustForForeignAbiError {
/// Target architecture doesn't support "foreign" (i.e. non-Rust) ABIs.
Unsupported { arch: Symbol, abi: spec::abi::Abi },
Unsupported { arch: Symbol, abi: ExternAbi },
}

impl<'a, Ty> FnAbi<'a, Ty> {
pub fn adjust_for_foreign_abi<C>(
&mut self,
cx: &C,
abi: spec::abi::Abi,
abi: ExternAbi,
) -> Result<(), AdjustForForeignAbiError>
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout + HasTargetSpec + HasWasmCAbiOpt + HasX86AbiOpt,
{
if abi == spec::abi::Abi::X86Interrupt {
if abi == ExternAbi::X86Interrupt {
if let Some(arg) = self.args.first_mut() {
arg.pass_by_stack_offset(None);
}
Expand All @@ -651,12 +650,10 @@ impl<'a, Ty> FnAbi<'a, Ty> {
match &spec.arch[..] {
"x86" => {
let (flavor, regparm) = match abi {
spec::abi::Abi::Fastcall { .. } | spec::abi::Abi::Vectorcall { .. } => {
ExternAbi::Fastcall { .. } | ExternAbi::Vectorcall { .. } => {
(x86::Flavor::FastcallOrVectorcall, None)
}
spec::abi::Abi::C { .. }
| spec::abi::Abi::Cdecl { .. }
| spec::abi::Abi::Stdcall { .. } => {
ExternAbi::C { .. } | ExternAbi::Cdecl { .. } | ExternAbi::Stdcall { .. } => {
(x86::Flavor::General, cx.x86_abi_opt().regparm)
}
_ => (x86::Flavor::General, None),
Expand All @@ -666,8 +663,10 @@ impl<'a, Ty> FnAbi<'a, Ty> {
x86::compute_abi_info(cx, self, opts);
}
"x86_64" => match abi {
spec::abi::Abi::SysV64 { .. } => x86_64::compute_abi_info(cx, self),
spec::abi::Abi::Win64 { .. } => x86_win64::compute_abi_info(cx, self),
ExternAbi::SysV64 { .. } => x86_64::compute_abi_info(cx, self),
ExternAbi::Win64 { .. } | ExternAbi::Vectorcall { .. } => {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the one actual change in this file: we treat Vectorcall as x86_win64 even on non-windows targets.

x86_win64::compute_abi_info(cx, self)
}
_ => {
if cx.target_spec().is_like_windows {
x86_win64::compute_abi_info(cx, self)
Expand Down Expand Up @@ -701,7 +700,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
"sparc" => sparc::compute_abi_info(cx, self),
"sparc64" => sparc64::compute_abi_info(cx, self),
"nvptx64" => {
if cx.target_spec().adjust_abi(abi, self.c_variadic) == spec::abi::Abi::PtxKernel {
if cx.target_spec().adjust_abi(abi, self.c_variadic) == ExternAbi::PtxKernel {
nvptx64::compute_ptx_kernel_abi_info(cx, self)
} else {
nvptx64::compute_abi_info(self)
Expand Down Expand Up @@ -730,7 +729,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
Ok(())
}

pub fn adjust_for_rust_abi<C>(&mut self, cx: &C, abi: SpecAbi)
pub fn adjust_for_rust_abi<C>(&mut self, cx: &C, abi: ExternAbi)
where
Ty: TyAbiInterface<'a, C> + Copy,
C: HasDataLayout + HasTargetSpec,
Expand Down Expand Up @@ -821,7 +820,7 @@ impl<'a, Ty> FnAbi<'a, Ty> {
// that's how we connect up to LLVM and it's unstable
// anyway, we control all calls to it in libstd.
BackendRepr::Vector { .. }
if abi != SpecAbi::RustIntrinsic && spec.simd_types_indirect =>
if abi != ExternAbi::RustIntrinsic && spec.simd_types_indirect =>
{
arg.make_indirect();
continue;
Expand Down
20 changes: 11 additions & 9 deletions compiler/rustc_target/src/callconv/x86_win64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::spec::HasTargetSpec;

// Win64 ABI: https://docs.microsoft.com/en-us/cpp/build/parameter-passing

pub(crate) fn compute_abi_info<Ty>(cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<'_, Ty>) {
pub(crate) fn compute_abi_info<Ty>(_cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<'_, Ty>) {
let fixup = |a: &mut ArgAbi<'_, Ty>| {
match a.layout.backend_repr {
BackendRepr::Uninhabited | BackendRepr::Memory { sized: false } => {}
Expand Down Expand Up @@ -40,16 +40,18 @@ pub(crate) fn compute_abi_info<Ty>(cx: &impl HasTargetSpec, fn_abi: &mut FnAbi<'
fixup(&mut fn_abi.ret);
}
for arg in fn_abi.args.iter_mut() {
if arg.is_ignore() {
// x86_64-pc-windows-gnu doesn't ignore ZSTs.
if cx.target_spec().os == "windows"
&& cx.target_spec().env == "gnu"
&& arg.layout.is_zst()
{
arg.make_indirect_from_ignore();
}
if arg.is_ignore() && arg.layout.is_zst() {
// Windows ABIs do not talk about ZST since such types do not exist in MSVC.
// In that sense we can do whatever we want here, and maybe we should throw an error
// (but of course that would be a massive breaking change now).
// We try to match clang and gcc (which allow ZST is their windows-gnu targets), so we
// pass ZST via pointer indirection.
arg.make_indirect_from_ignore();
continue;
}
fixup(arg);
}
// FIXME: We should likely also do something about ZST return types, similar to above.
// However, that's non-trivial due to `()`.
// See <https://github.com/rust-lang/unsafe-code-guidelines/issues/552>.
}
15 changes: 10 additions & 5 deletions compiler/rustc_target/src/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2815,12 +2815,17 @@ impl Target {
Abi::EfiApi if self.arch == "x86_64" => Abi::Win64 { unwind: false },
Abi::EfiApi => Abi::C { unwind: false },

// See commentary in `is_abi_supported`.
Abi::Stdcall { .. } | Abi::Thiscall { .. } if self.arch == "x86" => abi,
Abi::Stdcall { unwind } | Abi::Thiscall { unwind } => Abi::C { unwind },
Abi::Fastcall { .. } if self.arch == "x86" => abi,
// See commentary in `is_abi_supported`: we map these ABIs to "C" when they do not make sense.
Abi::Stdcall { .. } | Abi::Thiscall { .. } | Abi::Fastcall { .. }
if self.arch == "x86" =>
{
abi
}
Abi::Vectorcall { .. } if ["x86", "x86_64"].contains(&&self.arch[..]) => abi,
Abi::Fastcall { unwind } | Abi::Vectorcall { unwind } => Abi::C { unwind },
Abi::Stdcall { unwind }
| Abi::Thiscall { unwind }
| Abi::Fastcall { unwind }
| Abi::Vectorcall { unwind } => Abi::C { unwind },

// The Windows x64 calling convention we use for `extern "Rust"`
// <https://learn.microsoft.com/en-us/cpp/build/x64-software-conventions#register-volatility-and-preservation>
Expand Down
52 changes: 52 additions & 0 deletions tests/codegen/abi-win64-zst.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//@ compile-flags: -Z merge-functions=disabled

//@ revisions: windows-gnu
//@[windows-gnu] compile-flags: --target x86_64-pc-windows-gnu
//@[windows-gnu] needs-llvm-components: x86

//@ revisions: windows-msvc
//@[windows-msvc] compile-flags: --target x86_64-pc-windows-msvc
//@[windows-msvc] needs-llvm-components: x86

// Also test what happens when using a Windows ABI on Linux.
//@ revisions: linux
//@[linux] compile-flags: --target x86_64-unknown-linux-gnu
//@[linux] needs-llvm-components: x86

#![feature(no_core, lang_items, rustc_attrs, abi_vectorcall)]
#![no_core]
#![crate_type = "lib"]

#[lang = "sized"]
trait Sized {}

// Make sure the argument is always passed when explicitly requesting a Windows ABI.
// Our goal here is to match clang: <https://clang.godbolt.org/z/Wr4jMWq3P>.

// CHECK: define win64cc void @pass_zst_win64(ptr {{[^,]*}})
#[no_mangle]
extern "win64" fn pass_zst_win64(_: ()) {}

// CHECK: define x86_vectorcallcc void @pass_zst_vectorcall(ptr {{[^,]*}})
#[no_mangle]
extern "vectorcall" fn pass_zst_vectorcall(_: ()) {}

// windows-gnu: define void @pass_zst_fastcall(ptr {{[^,]*}})
// windows-msvc: define void @pass_zst_fastcall(ptr {{[^,]*}})
#[no_mangle]
#[cfg(windows)] // "fastcall" is not valid on 64bit Linux
extern "fastcall" fn pass_zst_fastcall(_: ()) {}

// The sysv64 ABI ignores ZST.

// CHECK: define x86_64_sysvcc void @pass_zst_sysv64()
#[no_mangle]
extern "sysv64" fn pass_zst_sysv64(_: ()) {}

// For `extern "C"` functions, ZST are ignored on Linux put passed on Windows.

// linux: define void @pass_zst_c()
// windows-msvc: define void @pass_zst_c(ptr {{[^,]*}})
// windows-gnu: define void @pass_zst_c(ptr {{[^,]*}})
#[no_mangle]
extern "C" fn pass_zst_c(_: ()) {}
24 changes: 0 additions & 24 deletions tests/ui/abi/win64-zst.rs

This file was deleted.

69 changes: 0 additions & 69 deletions tests/ui/abi/win64-zst.x86_64-linux.stderr

This file was deleted.

80 changes: 0 additions & 80 deletions tests/ui/abi/win64-zst.x86_64-windows-gnu.stderr

This file was deleted.

Loading
Loading