Skip to content

Commit

Permalink
Take CodegenFnAttrs into account when validating asm! register operands
Browse files Browse the repository at this point in the history
Checking of asm! register operands now properly takes function
attributes such as #[target_feature] and #[instruction_set] into
account.
  • Loading branch information
Amanieu committed Feb 21, 2022
1 parent 1ceb104 commit fc41d4b
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 187 deletions.
28 changes: 7 additions & 21 deletions compiler/rustc_ast_lowering/src/asm.rs
Expand Up @@ -64,13 +64,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let mut clobber_abis = FxHashMap::default();
if let Some(asm_arch) = asm_arch {
for (abi_name, abi_span) in &asm.clobber_abis {
match asm::InlineAsmClobberAbi::parse(
asm_arch,
self.sess.relocation_model(),
&self.sess.target_features,
&self.sess.target,
*abi_name,
) {
match asm::InlineAsmClobberAbi::parse(asm_arch, &self.sess.target, *abi_name) {
Ok(abi) => {
// If the abi was already in the list, emit an error
match clobber_abis.get(&abi) {
Expand Down Expand Up @@ -130,18 +124,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
.operands
.iter()
.map(|(op, op_sp)| {
let lower_reg = |reg, is_clobber| match reg {
let lower_reg = |reg| match reg {
InlineAsmRegOrRegClass::Reg(s) => {
asm::InlineAsmRegOrRegClass::Reg(if let Some(asm_arch) = asm_arch {
asm::InlineAsmReg::parse(
asm_arch,
sess.relocation_model(),
&sess.target_features,
&sess.target,
is_clobber,
s,
)
.unwrap_or_else(|e| {
asm::InlineAsmReg::parse(asm_arch, s).unwrap_or_else(|e| {
let msg = format!("invalid register `{}`: {}", s.as_str(), e);
sess.struct_span_err(*op_sp, &msg).emit();
asm::InlineAsmReg::Err
Expand All @@ -165,24 +151,24 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

let op = match *op {
InlineAsmOperand::In { reg, ref expr } => hir::InlineAsmOperand::In {
reg: lower_reg(reg, false),
reg: lower_reg(reg),
expr: self.lower_expr_mut(expr),
},
InlineAsmOperand::Out { reg, late, ref expr } => hir::InlineAsmOperand::Out {
reg: lower_reg(reg, expr.is_none()),
reg: lower_reg(reg),
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, false),
reg: lower_reg(reg),
late,
expr: self.lower_expr_mut(expr),
}
}
InlineAsmOperand::SplitInOut { reg, late, ref in_expr, ref out_expr } => {
hir::InlineAsmOperand::SplitInOut {
reg: lower_reg(reg, false),
reg: lower_reg(reg),
late,
in_expr: self.lower_expr_mut(in_expr),
out_expr: out_expr.as_ref().map(|expr| self.lower_expr_mut(expr)),
Expand Down
16 changes: 6 additions & 10 deletions compiler/rustc_codegen_cranelift/src/inline_asm.rs
Expand Up @@ -106,6 +106,7 @@ pub(crate) fn codegen_inline_asm<'tcx>(
let mut asm_gen = InlineAssemblyGenerator {
tcx: fx.tcx,
arch: fx.tcx.sess.asm_arch.unwrap(),
enclosing_def_id: fx.instance.def_id(),
template,
operands,
options,
Expand Down Expand Up @@ -169,6 +170,7 @@ pub(crate) fn codegen_inline_asm<'tcx>(
struct InlineAssemblyGenerator<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
arch: InlineAsmArch,
enclosing_def_id: DefId,
template: &'a [InlineAsmTemplatePiece],
operands: &'a [InlineAsmOperand<'tcx>],
options: InlineAsmOptions,
Expand All @@ -185,7 +187,7 @@ impl<'tcx> InlineAssemblyGenerator<'_, 'tcx> {
let map = allocatable_registers(
self.arch,
sess.relocation_model(),
&sess.target_features,
self.tcx.asm_target_features(self.enclosing_def_id),
&sess.target,
);
let mut allocated = FxHashMap::<_, (bool, bool)>::default();
Expand Down Expand Up @@ -318,15 +320,9 @@ impl<'tcx> InlineAssemblyGenerator<'_, 'tcx> {
let mut new_slot = |x| new_slot_fn(&mut slot_size, x);

// Allocate stack slots for saving clobbered registers
let abi_clobber = InlineAsmClobberAbi::parse(
self.arch,
self.tcx.sess.relocation_model(),
&self.tcx.sess.target_features,
&self.tcx.sess.target,
sym::C,
)
.unwrap()
.clobbered_regs();
let abi_clobber = InlineAsmClobberAbi::parse(self.arch, &self.tcx.sess.target, sym::C)
.unwrap()
.clobbered_regs();
for (i, reg) in self.registers.iter().enumerate().filter_map(|(i, r)| r.map(|r| (i, r))) {
let mut need_save = true;
// If the register overlaps with a register clobbered by function call, then
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Expand Up @@ -1046,6 +1046,10 @@ rustc_queries! {
cache_on_disk_if { true }
}

query asm_target_features(def_id: DefId) -> &'tcx FxHashSet<Symbol> {
desc { |tcx| "computing target features for inline asm of `{}`", tcx.def_path_str(def_id) }
}

query fn_arg_names(def_id: DefId) -> &'tcx [rustc_span::symbol::Ident] {
desc { |tcx| "looking up function parameter names for `{}`", tcx.def_path_str(def_id) }
separate_provide_extern
Expand Down
43 changes: 28 additions & 15 deletions compiler/rustc_passes/src/intrinsicck.rs
@@ -1,4 +1,5 @@
use rustc_ast::InlineAsmTemplatePiece;
use rustc_data_structures::stable_set::FxHashSet;
use rustc_errors::struct_span_err;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
Expand Down Expand Up @@ -138,7 +139,7 @@ impl<'tcx> ExprVisitor<'tcx> {
template: &[InlineAsmTemplatePiece],
is_input: bool,
tied_input: Option<(&hir::Expr<'tcx>, Option<InlineAsmType>)>,
target_features: &[Symbol],
target_features: &FxHashSet<Symbol>,
) -> Option<InlineAsmType> {
// Check the type against the allowed types for inline asm.
let ty = self.typeck_results.expr_ty_adjusted(expr);
Expand Down Expand Up @@ -285,9 +286,7 @@ impl<'tcx> ExprVisitor<'tcx> {
// (!). In that case we still need the earlier check to verify that the
// register class is usable at all.
if let Some(feature) = feature {
if !self.tcx.sess.target_features.contains(&feature)
&& !target_features.contains(&feature)
{
if !target_features.contains(&feature) {
let msg = &format!("`{}` target feature is not enabled", feature);
let mut err = self.tcx.sess.struct_span_err(expr.span, msg);
err.note(&format!(
Expand Down Expand Up @@ -347,7 +346,8 @@ impl<'tcx> ExprVisitor<'tcx> {
let hir = self.tcx.hir();
let enclosing_id = hir.enclosing_body_owner(hir_id);
let enclosing_def_id = hir.local_def_id(enclosing_id).to_def_id();
let attrs = self.tcx.codegen_fn_attrs(enclosing_def_id);
let target_features = self.tcx.asm_target_features(enclosing_def_id);
let asm_arch = self.tcx.sess.asm_arch.unwrap();
for (idx, (op, op_sp)) in asm.operands.iter().enumerate() {
// Validate register classes against currently enabled target
// features. We check that at least one type is available for
Expand All @@ -360,16 +360,29 @@ impl<'tcx> ExprVisitor<'tcx> {
// Note that this is only possible for explicit register
// operands, which cannot be used in the asm string.
if let Some(reg) = op.reg() {
// Some explicit registers cannot be used depending on the
// target. Reject those here.
if let InlineAsmRegOrRegClass::Reg(reg) = reg {
if let Err(msg) = reg.validate(
asm_arch,
self.tcx.sess.relocation_model(),
&target_features,
&self.tcx.sess.target,
op.is_clobber(),
) {
let msg = format!("cannot use register `{}`: {}", reg.name(), msg);
self.tcx.sess.struct_span_err(*op_sp, &msg).emit();
continue;
}
}

if !op.is_clobber() {
let mut missing_required_features = vec![];
let reg_class = reg.reg_class();
for &(_, feature) in reg_class.supported_types(self.tcx.sess.asm_arch.unwrap())
{
for &(_, feature) in reg_class.supported_types(asm_arch) {
match feature {
Some(feature) => {
if self.tcx.sess.target_features.contains(&feature)
|| attrs.target_features.contains(&feature)
{
if target_features.contains(&feature) {
missing_required_features.clear();
break;
} else {
Expand Down Expand Up @@ -425,7 +438,7 @@ impl<'tcx> ExprVisitor<'tcx> {
asm.template,
true,
None,
&attrs.target_features,
&target_features,
);
}
hir::InlineAsmOperand::Out { reg, late: _, ref expr } => {
Expand All @@ -437,7 +450,7 @@ impl<'tcx> ExprVisitor<'tcx> {
asm.template,
false,
None,
&attrs.target_features,
&target_features,
);
}
}
Expand All @@ -449,7 +462,7 @@ impl<'tcx> ExprVisitor<'tcx> {
asm.template,
false,
None,
&attrs.target_features,
&target_features,
);
}
hir::InlineAsmOperand::SplitInOut { reg, late: _, ref in_expr, ref out_expr } => {
Expand All @@ -460,7 +473,7 @@ impl<'tcx> ExprVisitor<'tcx> {
asm.template,
true,
None,
&attrs.target_features,
&target_features,
);
if let Some(out_expr) = out_expr {
self.check_asm_operand_type(
Expand All @@ -470,7 +483,7 @@ impl<'tcx> ExprVisitor<'tcx> {
asm.template,
false,
Some((in_expr, in_ty)),
&attrs.target_features,
&target_features,
);
}
}
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_target/src/asm/aarch64.rs
@@ -1,5 +1,5 @@
use super::{InlineAsmArch, InlineAsmType};
use crate::spec::{Target, RelocModel};
use crate::spec::{RelocModel, Target};
use rustc_data_structures::stable_set::FxHashSet;
use rustc_macros::HashStable_Generic;
use rustc_span::Symbol;
Expand Down Expand Up @@ -73,18 +73,18 @@ impl AArch64InlineAsmRegClass {
}
}

pub fn reserved_x18(
pub fn target_reserves_x18(target: &Target) -> bool {
target.os == "android" || target.is_like_fuchsia || target.is_like_osx || target.is_like_windows
}

fn reserved_x18(
_arch: InlineAsmArch,
_reloc_model: RelocModel,
_target_features: &FxHashSet<Symbol>,
target: &Target,
_is_clobber: bool,
) -> Result<(), &'static str> {
if target.os == "android"
|| target.is_like_fuchsia
|| target.is_like_osx
|| target.is_like_windows
{
if target_reserves_x18(target) {
Err("x18 is a reserved register on this target")
} else {
Ok(())
Expand Down

0 comments on commit fc41d4b

Please sign in to comment.