Skip to content

Commit

Permalink
Merge pull request #628 from folkertdev/clobber-input-registers
Browse files Browse the repository at this point in the history
fix clobbered or lateout registers overlapping with input registers
  • Loading branch information
antoyo authored Feb 13, 2025
2 parents b08a0c6 + 3ecc012 commit 1f98329
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 28 deletions.
2 changes: 1 addition & 1 deletion libgccjit.version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
e607be166673a8de9fc07f6f02c60426e556c5f2
d6f5a708104a98199ac0f01a3b6b279a0f7c66d3
74 changes: 48 additions & 26 deletions src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ use crate::type_of::LayoutGccExt;
//
// 3. Clobbers. GCC has a separate list of clobbers, and clobbers don't have indexes.
// Contrary, Rust expresses clobbers through "out" operands that aren't tied to
// a variable (`_`), and such "clobbers" do have index.
// a variable (`_`), and such "clobbers" do have index. Input operands cannot also
// be clobbered.
//
// 4. Furthermore, GCC Extended Asm does not support explicit register constraints
// (like `out("eax")`) directly, offering so-called "local register variables"
Expand Down Expand Up @@ -161,6 +162,16 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
// Also, we don't emit any asm operands immediately; we save them to
// the one of the buffers to be emitted later.

let mut input_registers = vec![];

for op in rust_operands {
if let InlineAsmOperandRef::In { reg, .. } = *op {
if let ConstraintOrRegister::Register(reg_name) = reg_to_gcc(reg) {
input_registers.push(reg_name);
}
}
}

// 1. Normal variables (and saving operands to buffers).
for (rust_idx, op) in rust_operands.iter().enumerate() {
match *op {
Expand All @@ -183,25 +194,39 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
continue;
}
(Register(reg_name), None) => {
// `clobber_abi` can add lots of clobbers that are not supported by the target,
// such as AVX-512 registers, so we just ignore unsupported registers
let is_target_supported =
reg.reg_class().supported_types(asm_arch, true).iter().any(
|&(_, feature)| {
if let Some(feature) = feature {
self.tcx
.asm_target_features(instance.def_id())
.contains(&feature)
} else {
true // Register class is unconditionally supported
}
},
);

if is_target_supported && !clobbers.contains(&reg_name) {
clobbers.push(reg_name);
if input_registers.contains(&reg_name) {
// the `clobber_abi` operand is converted into a series of
// `lateout("reg") _` operands. Of course, a user could also
// explicitly define such an output operand.
//
// GCC does not allow input registers to be clobbered, so if this out register
// is also used as an in register, do not add it to the clobbers list.
// it will be treated as a lateout register with `out_place: None`
if !late {
bug!("input registers can only be used as lateout regisers");
}
("r", dummy_output_type(self.cx, reg.reg_class()))
} else {
// `clobber_abi` can add lots of clobbers that are not supported by the target,
// such as AVX-512 registers, so we just ignore unsupported registers
let is_target_supported =
reg.reg_class().supported_types(asm_arch, true).iter().any(
|&(_, feature)| {
if let Some(feature) = feature {
self.tcx
.asm_target_features(instance.def_id())
.contains(&feature)
} else {
true // Register class is unconditionally supported
}
},
);

if is_target_supported && !clobbers.contains(&reg_name) {
clobbers.push(reg_name);
}
continue;
}
continue;
}
};

Expand Down Expand Up @@ -230,13 +255,10 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
}

InlineAsmOperandRef::InOut { reg, late, in_value, out_place } => {
let constraint =
if let ConstraintOrRegister::Constraint(constraint) = reg_to_gcc(reg) {
constraint
} else {
// left for the next pass
continue;
};
let ConstraintOrRegister::Constraint(constraint) = reg_to_gcc(reg) else {
// left for the next pass
continue;
};

// Rustc frontend guarantees that input and output types are "compatible",
// so we can just use input var's type for the output variable.
Expand Down
1 change: 0 additions & 1 deletion tests/failing-ui-tests.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
tests/ui/allocator/no_std-alloc-error-handler-custom.rs
tests/ui/allocator/no_std-alloc-error-handler-default.rs
tests/ui/asm/may_unwind.rs
tests/ui/asm/x86_64/multiple-clobber-abi.rs
tests/ui/functions-closures/parallel-codegen-closures.rs
tests/ui/linkage-attr/linkage1.rs
tests/ui/lto/dylib-works.rs
Expand Down
31 changes: 31 additions & 0 deletions tests/run/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,37 @@ fn asm() {
mem_cpy(array2.as_mut_ptr(), array1.as_ptr(), 3);
}
assert_eq!(array1, array2);

// in and clobber registers cannot overlap. This tests that the lateout register without an
// output place (indicated by the `_`) is not added to the list of clobbered registers
let x = 8;
let y: i32;
unsafe {
asm!(
"mov rax, rdi",
in("rdi") x,
lateout("rdi") _,
out("rax") y,
);
}
assert_eq!((x, y), (8, 8));

// sysv64 is the default calling convention on unix systems. The rdi register is
// used to pass arguments in the sysv64 calling convention, so this register will be clobbered
#[cfg(unix)]
{
let x = 16;
let y: i32;
unsafe {
asm!(
"mov rax, rdi",
in("rdi") x,
out("rax") y,
clobber_abi("sysv64"),
);
}
assert_eq!((x, y), (16, 16));
}
}

#[cfg(not(target_arch = "x86_64"))]
Expand Down

0 comments on commit 1f98329

Please sign in to comment.