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.

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

Jump to bottom

asm: Allow the use of r8-r14 as clobbers on Thumb1 #93877

Merged
merged 1 commit into from
Feb 19, 2022
Merged

asm: Allow the use of r8-r14 as clobbers on Thumb1 #93877

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
11 changes: 6 additions & 5 deletions compiler/rustc_ast_lowering/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
.operands
.iter()
.map(|(op, op_sp)| {
let lower_reg = |reg| match reg {
let lower_reg = |reg, is_clobber| match reg {
InlineAsmRegOrRegClass::Reg(s) => {
asm::InlineAsmRegOrRegClass::Reg(if let Some(asm_arch) = asm_arch {
asm::InlineAsmReg::parse(
asm_arch,
&sess.target_features,
&sess.target,
is_clobber,
s,
)
.unwrap_or_else(|e| {
Expand All @@ -162,24 +163,24 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

let op = match *op {
InlineAsmOperand::In { reg, ref expr } => hir::InlineAsmOperand::In {
reg: lower_reg(reg),
reg: lower_reg(reg, false),
expr: self.lower_expr_mut(expr),
},
InlineAsmOperand::Out { reg, late, ref expr } => hir::InlineAsmOperand::Out {
reg: lower_reg(reg),
reg: lower_reg(reg, expr.is_none()),
late,
expr: expr.as_ref().map(|expr| self.lower_expr_mut(expr)),
},
InlineAsmOperand::InOut { reg, late, ref expr } => {
hir::InlineAsmOperand::InOut {
reg: lower_reg(reg),
reg: lower_reg(reg, false),
late,
expr: self.lower_expr_mut(expr),
}
}
InlineAsmOperand::SplitInOut { reg, late, ref in_expr, ref out_expr } => {
hir::InlineAsmOperand::SplitInOut {
reg: lower_reg(reg),
reg: lower_reg(reg, false),
late,
in_expr: self.lower_expr_mut(in_expr),
out_expr: out_expr.as_ref().map(|expr| self.lower_expr_mut(expr)),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/asm/aarch64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pub fn reserved_x18(
_arch: InlineAsmArch,
_target_features: &FxHashSet<Symbol>,
target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
if target.os == "android"
|| target.is_like_fuchsia
Expand Down
17 changes: 13 additions & 4 deletions compiler/rustc_target/src/asm/arm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,13 @@ fn frame_pointer_is_r7(target_features: &FxHashSet<Symbol>, target: &Target) ->
}

fn frame_pointer_r11(
_arch: InlineAsmArch,
arch: InlineAsmArch,
target_features: &FxHashSet<Symbol>,
target: &Target,
is_clobber: bool,
) -> Result<(), &'static str> {
not_thumb1(arch, target_features, target, is_clobber)?;

if !frame_pointer_is_r7(target_features, target) {
Err("the frame pointer (r11) cannot be used as an operand for inline asm")
} else {
Expand All @@ -81,6 +84,7 @@ fn frame_pointer_r7(
_arch: InlineAsmArch,
target_features: &FxHashSet<Symbol>,
target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
if frame_pointer_is_r7(target_features, target) {
Err("the frame pointer (r7) cannot be used as an operand for inline asm")
Expand All @@ -93,9 +97,13 @@ fn not_thumb1(
_arch: InlineAsmArch,
target_features: &FxHashSet<Symbol>,
_target: &Target,
is_clobber: bool,
) -> Result<(), &'static str> {
if target_features.contains(&sym::thumb_mode) && !target_features.contains(&sym::thumb2) {
Err("high registers (r8+) cannot be used in Thumb-1 code")
if !is_clobber
&& target_features.contains(&sym::thumb_mode)
&& !target_features.contains(&sym::thumb2)
{
Err("high registers (r8+) can only be used as clobbers in Thumb-1 code")
} else {
Ok(())
}
Expand All @@ -105,8 +113,9 @@ fn reserved_r9(
arch: InlineAsmArch,
target_features: &FxHashSet<Symbol>,
target: &Target,
is_clobber: bool,
) -> Result<(), &'static str> {
not_thumb1(arch, target_features, target)?;
not_thumb1(arch, target_features, target, is_clobber)?;

// We detect this using the reserved-r9 feature instead of using the target
// because the relocation model can be changed with compiler options.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/asm/bpf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ fn only_alu32(
_arch: InlineAsmArch,
target_features: &FxHashSet<Symbol>,
_target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
if !target_features.contains(&sym::alu32) {
Err("register can't be used without the `alu32` target feature")
Expand Down
108 changes: 71 additions & 37 deletions compiler/rustc_target/src/asm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,13 @@ macro_rules! def_regs {
_arch: super::InlineAsmArch,
_target_features: &rustc_data_structures::fx::FxHashSet<Symbol>,
_target: &crate::spec::Target,
_is_clobber: bool,
name: &str,
) -> Result<Self, &'static str> {
match name {
$(
$($alias)|* | $reg_name => {
$($filter(_arch, _target_features, _target)?;)?
$($filter(_arch, _target_features, _target, _is_clobber)?;)?
Ok(Self::$reg)
}
)*
Expand All @@ -112,7 +113,7 @@ macro_rules! def_regs {
#[allow(unused_imports)]
use super::{InlineAsmReg, InlineAsmRegClass};
$(
if $($filter(_arch, _target_features, _target).is_ok() &&)? true {
if $($filter(_arch, _target_features, _target, false).is_ok() &&)? true {
if let Some(set) = _map.get_mut(&InlineAsmRegClass::$arch($arch_regclass::$class)) {
set.insert(InlineAsmReg::$arch($arch_reg::$reg));
}
Expand Down Expand Up @@ -298,54 +299,87 @@ impl InlineAsmReg {
arch: InlineAsmArch,
target_features: &FxHashSet<Symbol>,
target: &Target,
is_clobber: bool,
Copy link
Member

@nagisa nagisa Feb 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder if it wouldn't make sense to proactively make this a tri-state enum describing the context (clobber, input, output). I can imagine it being useful when we decide to actually start adding constraint classes that are applicable to input or output only.

And today it would serve as a way to avoid bool proliferation.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a followup PR based on this one which is a pain to rebase. I'll make the change there.

name: Symbol,
) -> Result<Self, &'static str> {
// FIXME: use direct symbol comparison for register names
// Use `Symbol::as_str` instead of `Symbol::with` here because `has_feature` may access `Symbol`.
let name = name.as_str();
Ok(match arch {
InlineAsmArch::X86 | InlineAsmArch::X86_64 => {
Self::X86(X86InlineAsmReg::parse(arch, target_features, target, name)?)
Self::X86(X86InlineAsmReg::parse(arch, target_features, target, is_clobber, name)?)
}
InlineAsmArch::Arm => {
Self::Arm(ArmInlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::AArch64 => {
Self::AArch64(AArch64InlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => {
Self::RiscV(RiscVInlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::Nvptx64 => {
Self::Nvptx(NvptxInlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::PowerPC | InlineAsmArch::PowerPC64 => {
Self::PowerPC(PowerPCInlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::Hexagon => {
Self::Hexagon(HexagonInlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::Mips | InlineAsmArch::Mips64 => {
Self::Mips(MipsInlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::S390x => {
Self::S390x(S390xInlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::SpirV => {
Self::SpirV(SpirVInlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::Wasm32 | InlineAsmArch::Wasm64 => {
Self::Wasm(WasmInlineAsmReg::parse(arch, target_features, target, name)?)
Self::Arm(ArmInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?)
}
InlineAsmArch::AArch64 => Self::AArch64(AArch64InlineAsmReg::parse(
arch,
target_features,
target,
is_clobber,
name,
)?),
InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => Self::RiscV(
RiscVInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?,
),
InlineAsmArch::Nvptx64 => Self::Nvptx(NvptxInlineAsmReg::parse(
arch,
target_features,
target,
is_clobber,
name,
)?),
InlineAsmArch::PowerPC | InlineAsmArch::PowerPC64 => Self::PowerPC(
PowerPCInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?,
),
InlineAsmArch::Hexagon => Self::Hexagon(HexagonInlineAsmReg::parse(
arch,
target_features,
target,
is_clobber,
name,
)?),
InlineAsmArch::Mips | InlineAsmArch::Mips64 => Self::Mips(MipsInlineAsmReg::parse(
arch,
target_features,
target,
is_clobber,
name,
)?),
InlineAsmArch::S390x => Self::S390x(S390xInlineAsmReg::parse(
arch,
target_features,
target,
is_clobber,
name,
)?),
InlineAsmArch::SpirV => Self::SpirV(SpirVInlineAsmReg::parse(
arch,
target_features,
target,
is_clobber,
name,
)?),
InlineAsmArch::Wasm32 | InlineAsmArch::Wasm64 => Self::Wasm(WasmInlineAsmReg::parse(
arch,
target_features,
target,
is_clobber,
name,
)?),
InlineAsmArch::Bpf => {
Self::Bpf(BpfInlineAsmReg::parse(arch, target_features, target, name)?)
Self::Bpf(BpfInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?)
}
InlineAsmArch::Avr => {
Self::Avr(AvrInlineAsmReg::parse(arch, target_features, target, name)?)
}
InlineAsmArch::Msp430 => {
Self::Msp430(Msp430InlineAsmReg::parse(arch, target_features, target, name)?)
Self::Avr(AvrInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?)
}
InlineAsmArch::Msp430 => Self::Msp430(Msp430InlineAsmReg::parse(
arch,
target_features,
target,
is_clobber,
name,
)?),
})
}

Expand Down Expand Up @@ -844,7 +878,7 @@ impl InlineAsmClobberAbi {
},
InlineAsmArch::AArch64 => match name {
"C" | "system" | "efiapi" => {
Ok(if aarch64::reserved_x18(arch, target_features, target).is_err() {
Ok(if aarch64::reserved_x18(arch, target_features, target, true).is_err() {
InlineAsmClobberAbi::AArch64NoX18
} else {
InlineAsmClobberAbi::AArch64
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_target/src/asm/riscv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ fn not_e(
_arch: InlineAsmArch,
target_features: &FxHashSet<Symbol>,
_target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
if target_features.contains(&sym::e) {
Err("register can't be used with the `e` target feature")
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_target/src/asm/x86.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ fn x86_64_only(
arch: InlineAsmArch,
_target_features: &FxHashSet<Symbol>,
_target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
match arch {
InlineAsmArch::X86 => Err("register is only available on x86_64"),
Expand All @@ -153,6 +154,7 @@ fn high_byte(
arch: InlineAsmArch,
_target_features: &FxHashSet<Symbol>,
_target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
match arch {
InlineAsmArch::X86_64 => Err("high byte registers cannot be used as an operand on x86_64"),
Expand All @@ -164,6 +166,7 @@ fn rbx_reserved(
arch: InlineAsmArch,
_target_features: &FxHashSet<Symbol>,
_target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
match arch {
InlineAsmArch::X86 => Ok(()),
Expand All @@ -178,6 +181,7 @@ fn esi_reserved(
arch: InlineAsmArch,
_target_features: &FxHashSet<Symbol>,
_target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
match arch {
InlineAsmArch::X86 => {
Expand Down