Skip to content

Remove remaining traces of AT&T assembly syntax #911

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.

Sign up for GitHub

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
tgross35 merged 1 commit into from
Dec 5, 2025
Merged

Remove remaining traces of AT&T assembly syntax #911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 2 additions & 4 deletions compiler-builtins/src/int/specialized_div_rem/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,12 @@ unsafe fn u128_by_u64_div_rem(duo: u128, div: u64) -> (u64, u64) {
unsafe {
// divides the combined registers rdx:rax (`duo` is split into two 64 bit parts to do this)
// by `div`. The quotient is stored in rax and the remainder in rdx.
// FIXME: Use the Intel syntax once we drop LLVM 9 support on rust-lang/rust.
core::arch::asm!(
"div {0}",
in(reg) div,
inlateout("rax") duo_lo => quo,
inlateout("rdx") duo_hi => rem,
options(att_syntax, pure, nomem, nostack)
options(pure, nomem, nostack),
);
}
(quo, rem)
Expand Down Expand Up @@ -283,13 +282,12 @@ unsafe fn u64_by_u32_div_rem(duo: u64, div: u32) -> (u32, u32) {
unsafe {
// divides the combined registers rdx:rax (`duo` is split into two 32 bit parts to do this)
// by `div`. The quotient is stored in rax and the remainder in rdx.
// FIXME: Use the Intel syntax once we drop LLVM 9 support on rust-lang/rust.
core::arch::asm!(
"div {0}",
in(reg) div,
inlateout("rax") duo_lo => quo,
inlateout("rdx") duo_hi => rem,
options(att_syntax, pure, nomem, nostack)
options(pure, nomem, nostack),
);
}
(quo, rem)
Expand Down
52 changes: 25 additions & 27 deletions compiler-builtins/src/mem/x86_64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ use core::{intrinsics, mem};
#[inline(always)]
#[cfg(target_feature = "ermsb")]
pub unsafe fn copy_forward(dest: *mut u8, src: *const u8, count: usize) {
// FIXME: Use the Intel syntax once we drop LLVM 9 support on rust-lang/rust.
core::arch::asm!(
"repe movsb (%rsi), (%rdi)",
asm!(
"repe movsb [rdi], [rsi]",
Copy link
Contributor

@quaternic quaternic May 10, 2025

Choose a reason for hiding this comment

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

Suggested change
"repe movsb [rdi], [rsi]",
"rep movsb [rdi], [rsi]",

The repe mnemonic is for "repeat while equal" and only makes sense for the string comparison operations
https://www.felixcloutier.com/x86/rep:repe:repz:repne:repnz
(but the encoding is the same so I guess assemblers don't care)

inout("rcx") count => _,
inout("rdi") dest => _,
inout("rsi") src => _,
options(att_syntax, nostack, preserves_flags)
options(nostack, preserves_flags)
);
}

Expand All @@ -42,21 +41,21 @@ pub unsafe fn copy_forward(mut dest: *mut u8, mut src: *const u8, count: usize)
inout("ecx") pre_byte_count => _,
inout("rdi") dest => dest,
inout("rsi") src => src,
options(att_syntax, nostack, preserves_flags)
options(nostack, preserves_flags)
);
asm!(
"rep movsq",
inout("rcx") qword_count => _,
inout("rdi") dest => dest,
inout("rsi") src => src,
options(att_syntax, nostack, preserves_flags)
options(nostack, preserves_flags)
);
asm!(
"rep movsb",
inout("ecx") byte_count => _,
inout("rdi") dest => _,
inout("rsi") src => _,
options(att_syntax, nostack, preserves_flags)
options(nostack, preserves_flags)
);
}

Expand All @@ -67,14 +66,14 @@ pub unsafe fn copy_backward(dest: *mut u8, src: *const u8, count: usize) {
asm!(
"std",
"rep movsb",
"sub $7, %rsi",
"sub $7, %rdi",
"mov {qword_count:r}, %rcx",
"sub rsi, 7",
"sub rdi, 7",
"mov rcx, {qword_count:r}",
"rep movsq",
"test {pre_byte_count:e}, {pre_byte_count:e}",
"add $7, %rsi",
"add $7, %rdi",
"mov {pre_byte_count:e}, %ecx",
"add rsi, 7",
"add rdi, 7",
"mov ecx, {pre_byte_count:e}",
"rep movsb",
"cld",
pre_byte_count = in(reg) pre_byte_count,
Expand All @@ -83,20 +82,19 @@ pub unsafe fn copy_backward(dest: *mut u8, src: *const u8, count: usize) {
inout("rdi") dest.add(count - 1) => _,
inout("rsi") src.add(count - 1) => _,
// We modify flags, but we restore it afterwards
options(att_syntax, nostack, preserves_flags)
options(nostack, preserves_flags)
Copy link
Contributor

@quaternic quaternic May 10, 2025

Choose a reason for hiding this comment

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

This definitely shouldn't have preserves_flags. Flags are modified by each of add,sub,test. The comment refers to how std/cld are used to set/clear the direction flag, but that's separate since it must be cleared before exiting the inline assembly.

Actually, what is the point of the test instruction here? It only affects flags, and none of the subsequent instructions depend on any flags.

Copy link
Contributor

@tgross35 tgross35 Dec 5, 2025

Choose a reason for hiding this comment

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

I think it can be removed: ae557bd had an early exit but ef37a23 removed it.

Copy link
Contributor

@tgross35 tgross35 Dec 5, 2025

Choose a reason for hiding this comment

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

This diff gave me a similar question about

compiler-builtins/compiler-builtins/src/x86_64.rs

Lines 20 to 26 in e1d8a49

"2:",
"sub $0x1000,%rcx",
"test %rcx,(%rcx)",
"sub $0x1000,%rax",
"cmp $0x1000,%rax",
"ja 2b",
"1:",
. Aren't flags set by test overwritten by the sub then cmp before ja checks them?

Copy link
Contributor

@quaternic quaternic Dec 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, that looked weird at first, but I think the test there just used as a way to load from memory without modifying any registers (except flags). So the loop is just reading from memory at 4kB intervals for the side-effects only, which is exactly what a chkstk should be doing.

);
}

#[inline(always)]
#[cfg(target_feature = "ermsb")]
pub unsafe fn set_bytes(dest: *mut u8, c: u8, count: usize) {
// FIXME: Use the Intel syntax once we drop LLVM 9 support on rust-lang/rust.
core::arch::asm!(
"repe stosb %al, (%rdi)",
asm!(
"repe stosb [rdi], al",
Copy link
Contributor

@quaternic quaternic May 10, 2025

Choose a reason for hiding this comment

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

Suggested change
"repe stosb [rdi], al",
"rep stosb [rdi], al",

inout("rcx") count => _,
inout("rdi") dest => _,
inout("al") c => _,
options(att_syntax, nostack, preserves_flags)
options(nostack, preserves_flags)
)
}

Expand All @@ -111,21 +109,21 @@ pub unsafe fn set_bytes(mut dest: *mut u8, c: u8, count: usize) {
inout("ecx") pre_byte_count => _,
inout("rdi") dest => dest,
in("rax") c,
options(att_syntax, nostack, preserves_flags)
options(nostack, preserves_flags)
);
asm!(
"rep stosq",
inout("rcx") qword_count => _,
inout("rdi") dest => dest,
in("rax") c,
options(att_syntax, nostack, preserves_flags)
options(nostack, preserves_flags)
);
asm!(
"rep stosb",
inout("ecx") byte_count => _,
inout("rdi") dest => _,
in("rax") c,
options(att_syntax, nostack, preserves_flags)
options(nostack, preserves_flags)
);
}

Expand Down Expand Up @@ -212,10 +210,10 @@ pub unsafe fn c_string_length(mut s: *const core::ffi::c_char) -> usize {
let x = {
let r;
asm!(
"movdqa ({addr:r}), {dest}",
"movdqa {dest}, [{addr:r}]",
addr = in(reg) s,
dest = out(xmm_reg) r,
options(att_syntax, nostack),
options(nostack, preserves_flags),
);
r
};
Expand All @@ -232,10 +230,10 @@ pub unsafe fn c_string_length(mut s: *const core::ffi::c_char) -> usize {
let x = {
let r;
asm!(
"movdqa ({addr:r}), {dest}",
"movdqa {dest}, [{addr:r}]",
addr = in(reg) s,
dest = out(xmm_reg) r,
options(att_syntax, nostack),
options(nostack, preserves_flags),
);
r
};
Expand Down Expand Up @@ -277,10 +275,10 @@ pub unsafe fn c_string_length(mut s: *const core::ffi::c_char) -> usize {
let mut cs = {
let r: u64;
asm!(
"mov ({addr}), {dest}",
"mov {dest}, [{addr}]",
addr = in(reg) s,
dest = out(reg) r,
options(att_syntax, nostack),
options(nostack, preserves_flags),
);
r
};
Expand Down
Loading
Loading