Skip to content
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
18 changes: 18 additions & 0 deletions compiler/rustc_target/src/callconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,24 @@ impl<'a, Ty> FnAbi<'a, Ty> {
continue;
}

// Always extend `bool` in the Rust ABI
let extend_bool = |attrs: &mut ArgAttributes, scalar: Scalar| {
if scalar.is_bool() {
attrs.ext(ArgExtension::Zext);
}
};

if let PassMode::Direct(attrs) = &mut arg.mode
&& let BackendRepr::Scalar(scalar) = arg.layout.backend_repr
{
extend_bool(attrs, scalar);
} else if let PassMode::Pair(a_attrs, b_attrs) = &mut arg.mode
&& let BackendRepr::ScalarPair { a, b, b_offset: _ } = arg.layout.backend_repr
{
extend_bool(a_attrs, a);
extend_bool(b_attrs, b);
}
Comment on lines +754 to +770

@workingjubilee workingjubilee Jul 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added to address @nikic's review that I shouldn't be diffing so many unrelated tests. Actually doesn't, neat, shrinking the diff considerably. Oddly annoying to puzzle out as, even though we split a single Rust-sig argument into two LLVM-sig arguments, we don't model that (leaving that to cg_llvm, etc.), and we don't e.g. somehow bundle together PassMode::Pair and BackendRepr::ScalarPair even though PassMode::Pair being attached to anything else is... probably wildly incorrect. Probably will want to restructure these pieces later.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm, yeah, we wind up having to reinvent this thing all over the compiler. Bad!


if arg_idx.is_none()
&& arg.layout.size > Primitive::Pointer(AddressSpace::ZERO).size(cx) * 2
&& !matches!(
Expand Down
11 changes: 1 addition & 10 deletions compiler/rustc_ty_utils/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ use rustc_middle::ty::layout::{
use rustc_middle::ty::{self, InstanceKind, ShimKind, Ty, TyCtxt, Unnormalized};
use rustc_span::DUMMY_SP;
use rustc_span::def_id::DefId;
use rustc_target::callconv::{
AbiMap, ArgAbi, ArgAttribute, ArgAttributes, ArgExtension, FnAbi, PassMode,
};
use rustc_target::callconv::{AbiMap, ArgAbi, ArgAttribute, ArgAttributes, FnAbi, PassMode};
use tracing::debug;

pub(crate) fn provide(providers: &mut Providers) {
Expand Down Expand Up @@ -331,13 +329,6 @@ fn arg_attrs_for_rust_scalar<'tcx>(
) -> ArgAttributes {
let mut attrs = ArgAttributes::new();

// Booleans are always a noundef i1 that needs to be zero-extended.
if scalar.is_bool() {
attrs.ext(ArgExtension::Zext);
attrs.set(ArgAttribute::NoUndef);
return attrs;
}

Comment thread
workingjubilee marked this conversation as resolved.
if !scalar.is_uninit_valid() {
attrs.set(ArgAttribute::NoUndef);
}
Expand Down
41 changes: 41 additions & 0 deletions tests/assembly-llvm/aarch64/mask-bool-ffi-return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
//@ add-minicore
//@ assembly-output: emit-asm
//@ needs-llvm-components: aarch64
//@ compile-flags: -Copt-level=3 --target=aarch64-unknown-linux-gnu

// Previously, Rust used to assume omnipresent zero-extension on bool in FFI returns
// which resulted in LLVM assuming that a register did not require explicit masking (e.g. `and`)
// and could be correctly handled via `cmp w0, #0`.
// `cmp` looks at the full register, but only the 8 bits that contain the bool are specified,
// and this is particularly glaring in the event of a branch or csel based on this.
// Thus we are looking for explicit handling like `tst w0, #0x1` or `and w0, #0xFF`

// NOTE: simplifying this further is risky as `tbnz x0, #0, ...` only examines 1 bit,
// so LLVM will optimize it all away if there's an immediate jump to another function

#![crate_type = "lib"]
#![feature(no_core)]
#![no_core]

extern crate minicore;

#[repr(C)]
struct Bools {
a: bool,
b: bool,
}

#[link(name = "rust_test_helpers")]
unsafe extern "C" {
safe fn bools_get_first_bool(bools: Bools) -> bool;
}

// CHECK-LABEL: broken
pub fn broken() -> i32 {
let bools = Bools { a: false, b: true };
// CHECK: bl bools_get_first_bool
// CHECK-NOT: cmp
// CHECK: tst w0, #0x1
// CHECK-NOT: cmp
if bools_get_first_bool(bools) { 123 } else { 321 }
}
27 changes: 27 additions & 0 deletions tests/codegen-llvm/some-abis-do-extend-params-to-32-bits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ use minicore::*;
//
// ZERO/SIGN-EXTENDING TO 32 BITS NON-EXTENDING
// ============================== =======================
// x86_64: void @c_arg_bool(i1 zeroext %_a)
// i686: void @c_arg_bool(i1 zeroext %_a)
// aarch64-apple: void @c_arg_bool(i1 zeroext %_a)
// aarch64-windows: void @c_arg_bool(i1 %_a)
// aarch64-linux: void @c_arg_bool(i1 %_a)
// arm: void @c_arg_bool(i1 zeroext %_a)
// riscv: void @c_arg_bool(i1 zeroext %_a)
#[no_mangle]
pub extern "C" fn c_arg_bool(_a: bool) {}

// x86_64: void @c_arg_u8(i8 zeroext %_a)
// i686: void @c_arg_u8(i8 zeroext %_a)
// aarch64-apple: void @c_arg_u8(i8 zeroext %_a)
Expand Down Expand Up @@ -113,6 +123,18 @@ pub extern "C" fn c_arg_i32(_a: i32) {}
#[no_mangle]
pub extern "C" fn c_arg_i64(_a: i64) {}

// x86_64: zeroext i1 @c_ret_bool()
// i686: zeroext i1 @c_ret_bool()
// aarch64-apple: zeroext i1 @c_ret_bool()
// aarch64-windows: i1 @c_ret_bool()
// aarch64-linux: i1 @c_ret_bool()
// arm: zeroext i1 @c_ret_bool()
// riscv: zeroext i1 @c_ret_bool()
#[no_mangle]
pub extern "C" fn c_ret_bool() -> bool {
false
}

// x86_64: zeroext i8 @c_ret_u8()
// i686: zeroext i8 @c_ret_u8()
// aarch64-apple: zeroext i8 @c_ret_u8()
Expand Down Expand Up @@ -210,10 +232,13 @@ pub extern "C" fn c_ret_i64() -> i64 {
}

const C_SOURCE_FILE: &'static str = r##"
#include <stdbool.h>
#include <stdlib.h>
#include <stdint.h>
#include <stdio.h>

void c_arg_bool(bool _a) { }

void c_arg_u8(uint8_t _a) { }
void c_arg_u16(uint16_t _a) { }
void c_arg_u32(uint32_t _a) { }
Expand All @@ -224,6 +249,8 @@ void c_arg_i16(int16_t _a) { }
void c_arg_i32(int32_t _a) { }
void c_arg_i64(int64_t _a) { }

bool c_ret_bool() { return false; }

uint8_t c_ret_u8() { return 0; }
uint16_t c_ret_u16() { return 0; }
uint32_t c_ret_u32() { return 0; }
Expand Down
Loading