Skip to content

Commit

Permalink
Rollup merge of #73588 - Amanieu:thumb-fp, r=nagisa
Browse files Browse the repository at this point in the history
Fix handling of reserved registers for ARM inline asm

`r6` is now disallowed as an operand since LLVM sometimes uses it as a base pointer.

The check against using the frame pointer as an operand now takes the platform into account and will block either `r7` or `r11` as appropriate.

Fixes #73450

cc @cbiffle
  • Loading branch information
Manishearth authored Jun 26, 2020
2 parents 25671fa + a98868d commit 81d2d3c
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 20 deletions.
8 changes: 6 additions & 2 deletions src/doc/unstable-book/src/library-features/asm.md
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ Here is the list of currently supported register classes:
| AArch64 | `reg` | `x[0-28]`, `x30` | `r` |
| AArch64 | `vreg` | `v[0-31]` | `w` |
| AArch64 | `vreg_low16` | `v[0-15]` | `x` |
| ARM | `reg` | `r[0-r10]`, `r12`, `r14` | `r` |
| ARM | `reg` | `r[0-5]` `r7`\*, `r[8-10]`, `r11`\*, `r12`, `r14` | `r` |
| ARM (Thumb) | `reg_thumb` | `r[0-r7]` | `l` |
| ARM (ARM) | `reg_thumb` | `r[0-r10]`, `r12`, `r14` | `l` |
| ARM | `sreg` | `s[0-31]` | `t` |
Expand All @@ -497,6 +497,8 @@ Here is the list of currently supported register classes:
> Note #2: On x86-64 the high byte registers (e.g. `ah`) are only available when used as an explicit register. Specifying the `reg_byte` register class for an operand will always allocate a low byte register.
>
> Note #3: NVPTX doesn't have a fixed register set, so named registers are not supported.
>
> Note #4: On ARM the frame pointer is either `r7` or `r11` depending on the platform.
Additional register classes may be added in the future based on demand (e.g. MMX, x87, etc).

Expand Down Expand Up @@ -591,7 +593,9 @@ Some registers cannot be used for input or output operands:
| Architecture | Unsupported register | Reason |
| ------------ | -------------------- | ------ |
| All | `sp` | The stack pointer must be restored to its original value at the end of an asm code block. |
| All | `bp` (x86), `r11` (ARM), `x29` (AArch64), `x8` (RISC-V), `fr` (Hexagon) | The frame pointer cannot be used as an input or output. |
| All | `bp` (x86), `x29` (AArch64), `x8` (RISC-V), `fr` (Hexagon) | The frame pointer cannot be used as an input or output. |
| ARM | `r7` or `r11` | On ARM the frame pointer can be either `r7` or `r11` depending on the target. The frame pointer cannot be used as an input or output. |
| ARM | `r6` | `r6` is used internally by LLVM as a base pointer and therefore cannot be used as an input or output. |
| x86 | `k0` | This is a constant zero register which can't be modified. |
| x86 | `ip` | This is the program counter, not a real register. |
| x86 | `mm[0-7]` | MMX registers are not currently supported (but may be in the future). |
Expand Down
1 change: 1 addition & 0 deletions src/librustc_ast_lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
asm::InlineAsmReg::parse(
sess.asm_arch?,
|feature| sess.target_features.contains(&Symbol::intern(feature)),
&sess.target.target,
s,
)
.map_err(|e| {
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_codegen_llvm/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ const ARM_WHITELIST: &[(&str, Option<Symbol>)] = &[
("vfp2", Some(sym::arm_target_feature)),
("vfp3", Some(sym::arm_target_feature)),
("vfp4", Some(sym::arm_target_feature)),
// This is needed for inline assembly, but shouldn't be stabilized as-is
// since it should be enabled per-function using #[instruction_set], not
// #[target_feature].
("thumb-mode", Some(sym::arm_target_feature)),
];

const AARCH64_WHITELIST: &[(&str, Option<Symbol>)] = &[
Expand Down
40 changes: 36 additions & 4 deletions src/librustc_target/asm/arm.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::{InlineAsmArch, InlineAsmType};
use crate::spec::Target;
use rustc_macros::HashStable_Generic;
use std::fmt;

Expand Down Expand Up @@ -58,6 +59,37 @@ impl ArmInlineAsmRegClass {
}
}

// This uses the same logic as useR7AsFramePointer in LLVM
fn frame_pointer_is_r7(mut has_feature: impl FnMut(&str) -> bool, target: &Target) -> bool {
target.options.is_like_osx || (!target.options.is_like_windows && has_feature("thumb-mode"))
}

fn frame_pointer_r11(
_arch: InlineAsmArch,
has_feature: impl FnMut(&str) -> bool,
target: &Target,
_allocating: bool,
) -> Result<(), &'static str> {
if !frame_pointer_is_r7(has_feature, target) {
Err("the frame pointer (r11) cannot be used as an operand for inline asm")
} else {
Ok(())
}
}

fn frame_pointer_r7(
_arch: InlineAsmArch,
has_feature: impl FnMut(&str) -> bool,
target: &Target,
_allocating: bool,
) -> Result<(), &'static str> {
if frame_pointer_is_r7(has_feature, target) {
Err("the frame pointer (r7) cannot be used as an operand for inline asm")
} else {
Ok(())
}
}

def_regs! {
Arm ArmInlineAsmReg ArmInlineAsmRegClass {
r0: reg, reg_thumb = ["r0", "a1"],
Expand All @@ -66,11 +98,11 @@ def_regs! {
r3: reg, reg_thumb = ["r3", "a4"],
r4: reg, reg_thumb = ["r4", "v1"],
r5: reg, reg_thumb = ["r5", "v2"],
r6: reg, reg_thumb = ["r6", "v3"],
r7: reg, reg_thumb = ["r7", "v4"],
r7: reg, reg_thumb = ["r7", "v4"] % frame_pointer_r7,
r8: reg = ["r8", "v5"],
r9: reg = ["r9", "v6", "rfp"],
r10: reg = ["r10", "sl"],
r11: reg = ["r11", "fp"] % frame_pointer_r11,
r12: reg = ["r12", "ip"],
r14: reg = ["r14", "lr"],
s0: sreg, sreg_low16 = ["s0"],
Expand Down Expand Up @@ -153,8 +185,8 @@ def_regs! {
q13: qreg = ["q13"],
q14: qreg = ["q14"],
q15: qreg = ["q15"],
#error = ["r11", "fp"] =>
"the frame pointer cannot be used as an operand for inline asm",
#error = ["r6", "v3"] =>
"r6 is used internally by LLVM and cannot be used as an operand for inline asm",
#error = ["r13", "sp"] =>
"the stack pointer cannot be used as an operand for inline asm",
#error = ["r15", "pc"] =>
Expand Down
35 changes: 21 additions & 14 deletions src/librustc_target/asm/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::abi::Size;
use crate::spec::Target;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_macros::HashStable_Generic;
use rustc_span::Symbol;
Expand Down Expand Up @@ -83,12 +84,13 @@ macro_rules! def_regs {
pub fn parse(
_arch: super::InlineAsmArch,
mut _has_feature: impl FnMut(&str) -> bool,
_target: &crate::spec::Target,
name: &str,
) -> Result<Self, &'static str> {
match name {
$(
$($alias)|* | $reg_name => {
$($filter(_arch, &mut _has_feature, false)?;)?
$($filter(_arch, &mut _has_feature, _target, false)?;)?
Ok(Self::$reg)
}
)*
Expand All @@ -103,6 +105,7 @@ macro_rules! def_regs {
pub(super) fn fill_reg_map(
_arch: super::InlineAsmArch,
mut _has_feature: impl FnMut(&str) -> bool,
_target: &crate::spec::Target,
_map: &mut rustc_data_structures::fx::FxHashMap<
super::InlineAsmRegClass,
rustc_data_structures::fx::FxHashSet<super::InlineAsmReg>,
Expand All @@ -111,7 +114,7 @@ macro_rules! def_regs {
#[allow(unused_imports)]
use super::{InlineAsmReg, InlineAsmRegClass};
$(
if $($filter(_arch, &mut _has_feature, true).is_ok() &&)? true {
if $($filter(_arch, &mut _has_feature, _target, true).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 @@ -234,27 +237,30 @@ impl InlineAsmReg {
pub fn parse(
arch: InlineAsmArch,
has_feature: impl FnMut(&str) -> bool,
target: &Target,
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, has_feature, &name)?)
Self::X86(X86InlineAsmReg::parse(arch, has_feature, target, &name)?)
}
InlineAsmArch::Arm => {
Self::Arm(ArmInlineAsmReg::parse(arch, has_feature, target, &name)?)
}
InlineAsmArch::Arm => Self::Arm(ArmInlineAsmReg::parse(arch, has_feature, &name)?),
InlineAsmArch::AArch64 => {
Self::AArch64(AArch64InlineAsmReg::parse(arch, has_feature, &name)?)
Self::AArch64(AArch64InlineAsmReg::parse(arch, has_feature, target, &name)?)
}
InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => {
Self::RiscV(RiscVInlineAsmReg::parse(arch, has_feature, &name)?)
Self::RiscV(RiscVInlineAsmReg::parse(arch, has_feature, target, &name)?)
}
InlineAsmArch::Nvptx64 => {
Self::Nvptx(NvptxInlineAsmReg::parse(arch, has_feature, &name)?)
Self::Nvptx(NvptxInlineAsmReg::parse(arch, has_feature, target, &name)?)
}
InlineAsmArch::Hexagon => {
Self::Hexagon(HexagonInlineAsmReg::parse(arch, has_feature, &name)?)
Self::Hexagon(HexagonInlineAsmReg::parse(arch, has_feature, target, &name)?)
}
})
}
Expand Down Expand Up @@ -536,36 +542,37 @@ impl fmt::Display for InlineAsmType {
pub fn allocatable_registers(
arch: InlineAsmArch,
has_feature: impl FnMut(&str) -> bool,
target: &crate::spec::Target,
) -> FxHashMap<InlineAsmRegClass, FxHashSet<InlineAsmReg>> {
match arch {
InlineAsmArch::X86 | InlineAsmArch::X86_64 => {
let mut map = x86::regclass_map();
x86::fill_reg_map(arch, has_feature, &mut map);
x86::fill_reg_map(arch, has_feature, target, &mut map);
map
}
InlineAsmArch::Arm => {
let mut map = arm::regclass_map();
arm::fill_reg_map(arch, has_feature, &mut map);
arm::fill_reg_map(arch, has_feature, target, &mut map);
map
}
InlineAsmArch::AArch64 => {
let mut map = aarch64::regclass_map();
aarch64::fill_reg_map(arch, has_feature, &mut map);
aarch64::fill_reg_map(arch, has_feature, target, &mut map);
map
}
InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => {
let mut map = riscv::regclass_map();
riscv::fill_reg_map(arch, has_feature, &mut map);
riscv::fill_reg_map(arch, has_feature, target, &mut map);
map
}
InlineAsmArch::Nvptx64 => {
let mut map = nvptx::regclass_map();
nvptx::fill_reg_map(arch, has_feature, &mut map);
nvptx::fill_reg_map(arch, has_feature, target, &mut map);
map
}
InlineAsmArch::Hexagon => {
let mut map = hexagon::regclass_map();
hexagon::fill_reg_map(arch, has_feature, &mut map);
hexagon::fill_reg_map(arch, has_feature, target, &mut map);
map
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_target/asm/riscv.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::{InlineAsmArch, InlineAsmType};
use crate::spec::Target;
use rustc_macros::HashStable_Generic;
use std::fmt;

Expand Down Expand Up @@ -50,6 +51,7 @@ impl RiscVInlineAsmRegClass {
fn not_e(
_arch: InlineAsmArch,
mut has_feature: impl FnMut(&str) -> bool,
_target: &Target,
_allocating: bool,
) -> Result<(), &'static str> {
if has_feature("e") {
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_target/asm/x86.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::{InlineAsmArch, InlineAsmType};
use crate::spec::Target;
use rustc_macros::HashStable_Generic;
use std::fmt;

Expand Down Expand Up @@ -131,6 +132,7 @@ impl X86InlineAsmRegClass {
fn x86_64_only(
arch: InlineAsmArch,
_has_feature: impl FnMut(&str) -> bool,
_target: &Target,
_allocating: bool,
) -> Result<(), &'static str> {
match arch {
Expand All @@ -143,6 +145,7 @@ fn x86_64_only(
fn high_byte(
arch: InlineAsmArch,
_has_feature: impl FnMut(&str) -> bool,
_target: &Target,
allocating: bool,
) -> Result<(), &'static str> {
match arch {
Expand Down

0 comments on commit 81d2d3c

Please sign in to comment.