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

Implement sym operands for global_asm! #94468

Merged
merged 3 commits into from
Apr 16, 2022
Merged

Implement sym operands for global_asm! #94468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc345d8
Reimplement lowering of sym operands for asm! so that it also works w…
Amanieu Mar 1, 2022
547405e
Add codegen for global_asm! sym operands
Amanieu Mar 1, 2022
bdba897
Update tests for sym support in global_asm!
Amanieu Mar 1, 2022
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
16 changes: 15 additions & 1 deletion compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2061,6 +2061,20 @@ impl InlineAsmTemplatePiece {
}
}

/// Inline assembly symbol operands get their own AST node that is somewhat
/// similar to `AnonConst`.
///
/// The main difference is that we specifically don't assign it `DefId` in
/// `DefCollector`. Instead this is deferred until AST lowering where we
/// lower it to an `AnonConst` (for functions) or a `Path` (for statics)
/// depending on what the path resolves to.
#[derive(Clone, Encodable, Decodable, Debug)]
pub struct InlineAsmSym {
pub id: NodeId,
pub qself: Option<QSelf>,
pub path: Path,
}

/// Inline assembly operand.
///
/// E.g., `out("eax") result` as in `asm!("mov eax, 2", out("eax") result)`.
Expand Down Expand Up @@ -2090,7 +2104,7 @@ pub enum InlineAsmOperand {
anon_const: AnonConst,
},
Sym {
expr: P<Expr>,
sym: InlineAsmSym,
},
}

Expand Down
29 changes: 23 additions & 6 deletions compiler/rustc_ast/src/mut_visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,14 @@ pub trait MutVisitor: Sized {
fn flat_map_pat_field(&mut self, fp: PatField) -> SmallVec<[PatField; 1]> {
noop_flat_map_pat_field(fp, self)
}

fn visit_inline_asm(&mut self, asm: &mut InlineAsm) {
noop_visit_inline_asm(asm, self)
}

fn visit_inline_asm_sym(&mut self, sym: &mut InlineAsmSym) {
noop_visit_inline_asm_sym(sym, self)
}
}

/// Use a map-style function (`FnOnce(T) -> T`) to overwrite a `&mut T`. Useful
Expand Down Expand Up @@ -1019,7 +1027,7 @@ pub fn noop_visit_item_kind<T: MutVisitor>(kind: &mut ItemKind, vis: &mut T) {
}
}
ItemKind::ForeignMod(nm) => vis.visit_foreign_mod(nm),
ItemKind::GlobalAsm(asm) => noop_visit_inline_asm(asm, vis),
ItemKind::GlobalAsm(asm) => vis.visit_inline_asm(asm),
ItemKind::TyAlias(box TyAlias {
defaultness, generics, where_clauses, bounds, ty, ..
}) => {
Expand Down Expand Up @@ -1237,25 +1245,34 @@ pub fn noop_visit_anon_const<T: MutVisitor>(AnonConst { id, value }: &mut AnonCo
vis.visit_expr(value);
}

fn noop_visit_inline_asm<T: MutVisitor>(asm: &mut InlineAsm, vis: &mut T) {
pub fn noop_visit_inline_asm<T: MutVisitor>(asm: &mut InlineAsm, vis: &mut T) {
for (op, _) in &mut asm.operands {
match op {
InlineAsmOperand::In { expr, .. }
| InlineAsmOperand::Out { expr: Some(expr), .. }
| InlineAsmOperand::InOut { expr, .. }
| InlineAsmOperand::Sym { expr, .. } => vis.visit_expr(expr),
| InlineAsmOperand::InOut { expr, .. } => vis.visit_expr(expr),
InlineAsmOperand::Out { expr: None, .. } => {}
InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => {
vis.visit_expr(in_expr);
if let Some(out_expr) = out_expr {
vis.visit_expr(out_expr);
}
}
InlineAsmOperand::Const { anon_const, .. } => vis.visit_anon_const(anon_const),
InlineAsmOperand::Const { anon_const } => vis.visit_anon_const(anon_const),
InlineAsmOperand::Sym { sym } => vis.visit_inline_asm_sym(sym),
}
}
}

pub fn noop_visit_inline_asm_sym<T: MutVisitor>(
InlineAsmSym { id, qself, path }: &mut InlineAsmSym,
vis: &mut T,
) {
vis.visit_id(id);
vis.visit_qself(qself);
vis.visit_path(path);
}

pub fn noop_visit_expr<T: MutVisitor>(
Expr { kind, id, span, attrs, tokens }: &mut Expr,
vis: &mut T,
Expand Down Expand Up @@ -1374,7 +1391,7 @@ pub fn noop_visit_expr<T: MutVisitor>(
ExprKind::Ret(expr) => {
visit_opt(expr, |expr| vis.visit_expr(expr));
}
ExprKind::InlineAsm(asm) => noop_visit_inline_asm(asm, vis),
ExprKind::InlineAsm(asm) => vis.visit_inline_asm(asm),
ExprKind::MacCall(mac) => vis.visit_mac_call(mac),
ExprKind::Struct(se) => {
let StructExpr { qself, path, fields, rest } = se.deref_mut();
Expand Down
19 changes: 16 additions & 3 deletions compiler/rustc_ast/src/visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ pub trait Visitor<'ast>: Sized {
fn visit_crate(&mut self, krate: &'ast Crate) {
walk_crate(self, krate)
}
fn visit_inline_asm(&mut self, asm: &'ast InlineAsm) {
walk_inline_asm(self, asm)
}
fn visit_inline_asm_sym(&mut self, sym: &'ast InlineAsmSym) {
walk_inline_asm_sym(self, sym)
}
}

#[macro_export]
Expand Down Expand Up @@ -717,13 +723,12 @@ pub fn walk_anon_const<'a, V: Visitor<'a>>(visitor: &mut V, constant: &'a AnonCo
visitor.visit_expr(&constant.value);
}

fn walk_inline_asm<'a, V: Visitor<'a>>(visitor: &mut V, asm: &'a InlineAsm) {
pub fn walk_inline_asm<'a, V: Visitor<'a>>(visitor: &mut V, asm: &'a InlineAsm) {
for (op, _) in &asm.operands {
match op {
InlineAsmOperand::In { expr, .. }
| InlineAsmOperand::Out { expr: Some(expr), .. }
| InlineAsmOperand::InOut { expr, .. }
| InlineAsmOperand::Sym { expr, .. } => visitor.visit_expr(expr),
| InlineAsmOperand::InOut { expr, .. } => visitor.visit_expr(expr),
InlineAsmOperand::Out { expr: None, .. } => {}
InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => {
visitor.visit_expr(in_expr);
Expand All @@ -732,10 +737,18 @@ fn walk_inline_asm<'a, V: Visitor<'a>>(visitor: &mut V, asm: &'a InlineAsm) {
}
}
InlineAsmOperand::Const { anon_const, .. } => visitor.visit_anon_const(anon_const),
InlineAsmOperand::Sym { sym } => visitor.visit_inline_asm_sym(sym),
}
}
}

pub fn walk_inline_asm_sym<'a, V: Visitor<'a>>(visitor: &mut V, sym: &'a InlineAsmSym) {
if let Some(ref qself) = sym.qself {
visitor.visit_ty(&qself.ty);
}
visitor.visit_path(&sym.path, sym.id);
}

pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
walk_list!(visitor, visit_attribute, expression.attrs.iter());

Expand Down
65 changes: 60 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
@@ -1,12 +1,17 @@
use crate::{ImplTraitContext, ImplTraitPosition, ParamMode};

use super::LoweringContext;

use rustc_ast::ptr::P;
use rustc_ast::*;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_set::FxHashSet;
use rustc_errors::struct_span_err;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::definitions::DefPathData;
use rustc_session::parse::feature_err;
use rustc_span::{sym, Span};
use rustc_span::{sym, ExpnId, Span};
use rustc_target::asm;
use std::collections::hash_map::Entry;
use std::fmt::Write;
Expand Down Expand Up @@ -188,7 +193,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
anon_const: self.lower_anon_const(anon_const),
}
}
InlineAsmOperand::Sym { ref expr } => {
InlineAsmOperand::Sym { ref sym } => {
if !self.sess.features_untracked().asm_sym {
feature_err(
&self.sess.parse_sess,
Expand All @@ -198,7 +203,54 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
)
.emit();
}
hir::InlineAsmOperand::Sym { expr: self.lower_expr_mut(expr) }

let static_def_id = self
.resolver
.get_partial_res(sym.id)
.filter(|res| res.unresolved_segments() == 0)
.and_then(|res| {
if let Res::Def(DefKind::Static(_), def_id) = res.base_res() {
Some(def_id)
} else {
None
}
});

if let Some(def_id) = static_def_id {
let path = self.lower_qpath(
sym.id,
&sym.qself,
&sym.path,
ParamMode::Optional,
ImplTraitContext::Disallowed(ImplTraitPosition::Path),
);
hir::InlineAsmOperand::SymStatic { path, def_id }
} else {
// Replace the InlineAsmSym AST node with an
// Expr using the name node id.
let expr = Expr {
id: sym.id,
kind: ExprKind::Path(sym.qself.clone(), sym.path.clone()),
span: *op_sp,
attrs: AttrVec::new(),
tokens: None,
};

// Wrap the expression in an AnonConst.
let parent_def_id = self.current_hir_id_owner;
let node_id = self.resolver.next_node_id();
self.resolver.create_def(
parent_def_id,
node_id,
DefPathData::AnonConst,
ExpnId::root(),
*op_sp,
);
let anon_const = AnonConst { id: node_id, value: P(expr) };
hir::InlineAsmOperand::SymFn {
anon_const: self.lower_anon_const(&anon_const),
}
}
}
};
(op, self.lower_span(*op_sp))
Expand Down Expand Up @@ -260,7 +312,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
err.span_label(op_sp, "argument");
err.emit();
}
hir::InlineAsmOperand::Sym { .. } => {
hir::InlineAsmOperand::SymFn { .. }
| hir::InlineAsmOperand::SymStatic { .. } => {
let mut err = sess.struct_span_err(
placeholder_span,
"asm template modifiers are not allowed for `sym` arguments",
Expand Down Expand Up @@ -308,7 +361,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
hir::InlineAsmOperand::InOut { .. }
| hir::InlineAsmOperand::SplitInOut { .. } => (true, true),

hir::InlineAsmOperand::Const { .. } | hir::InlineAsmOperand::Sym { .. } => {
hir::InlineAsmOperand::Const { .. }
| hir::InlineAsmOperand::SymFn { .. }
| hir::InlineAsmOperand::SymStatic { .. } => {
unreachable!()
}
};
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_ast_pretty/src/pprust/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1266,10 +1266,14 @@ impl<'a> State<'a> {
s.space();
s.print_expr(&anon_const.value);
}
InlineAsmOperand::Sym { expr } => {
InlineAsmOperand::Sym { sym } => {
s.word("sym");
s.space();
s.print_expr(expr);
if let Some(qself) = &sym.qself {
s.print_qpath(&sym.path, qself, true);
} else {
s.print_path(&sym.path, true, 0);
}
}
}
}
Expand Down
22 changes: 12 additions & 10 deletions compiler/rustc_builtin_macros/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,17 +154,19 @@ pub fn parse_asm_args<'a>(
} else if p.eat_keyword(kw::Const) {
let anon_const = p.parse_anon_const_expr()?;
ast::InlineAsmOperand::Const { anon_const }
} else if !is_global_asm && p.eat_keyword(sym::sym) {
} else if p.eat_keyword(sym::sym) {
let expr = p.parse_expr()?;
match expr.kind {
ast::ExprKind::Path(..) => {}
_ => {
let err = diag
.struct_span_err(expr.span, "argument to `sym` must be a path expression");
return Err(err);
}
}
ast::InlineAsmOperand::Sym { expr }
let ast::ExprKind::Path(qself, path) = &expr.kind else {
let err = diag
.struct_span_err(expr.span, "expected a path for argument to `sym`");
return Err(err);
};
let sym = ast::InlineAsmSym {
id: ast::DUMMY_NODE_ID,
qself: qself.clone(),
path: path.clone(),
};
ast::InlineAsmOperand::Sym { sym }
} else if allow_templates {
let template = p.parse_expr()?;
// If it can't possibly expand to a string, provide diagnostics here to include other
Expand Down
32 changes: 28 additions & 4 deletions compiler/rustc_codegen_gcc/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,14 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
}

InlineAsmOperandRef::SymFn { instance } => {
// TODO(@Amanieu): Additional mangling is needed on
// some targets to add a leading underscore (Mach-O)
// or byte count suffixes (x86 Windows).
constants_len += self.tcx.symbol_name(instance).name.len();
}
InlineAsmOperandRef::SymStatic { def_id } => {
// TODO(@Amanieu): Additional mangling is needed on
// some targets to add a leading underscore (Mach-O).
constants_len += self.tcx.symbol_name(Instance::mono(self.tcx, def_id)).name.len();
}
}
Expand Down Expand Up @@ -412,13 +417,16 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
}

InlineAsmOperandRef::SymFn { instance } => {
// TODO(@Amanieu): Additional mangling is needed on
// some targets to add a leading underscore (Mach-O)
// or byte count suffixes (x86 Windows).
let name = self.tcx.symbol_name(instance).name;
template_str.push_str(name);
}

InlineAsmOperandRef::SymStatic { def_id } => {
// TODO(@Commeownist): This may not be sufficient for all kinds of statics.
// Some statics may need the `@plt` suffix, like thread-local vars.
// TODO(@Amanieu): Additional mangling is needed on
// some targets to add a leading underscore (Mach-O).
Copy link
Member

Choose a reason for hiding this comment

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

Does this change imply correct handling of @PLT and similar? If so, is there a test to that effect?

Copy link
Member Author

Choose a reason for hiding this comment

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

We never add @ suffixes to symbol names, it's the user's responsibility to add any necessary suffixes.

let instance = Instance::mono(self.tcx, def_id);
let name = self.tcx.symbol_name(instance).name;
template_str.push_str(name);
Expand Down Expand Up @@ -656,8 +664,8 @@ fn dummy_output_type<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, reg: InlineAsmRegCl
}
}

impl<'gcc, 'tcx> AsmMethods for CodegenCx<'gcc, 'tcx> {
fn codegen_global_asm(&self, template: &[InlineAsmTemplatePiece], operands: &[GlobalAsmOperandRef], options: InlineAsmOptions, _line_spans: &[Span]) {
impl<'gcc, 'tcx> AsmMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
fn codegen_global_asm(&self, template: &[InlineAsmTemplatePiece], operands: &[GlobalAsmOperandRef<'tcx>], options: InlineAsmOptions, _line_spans: &[Span]) {
let asm_arch = self.tcx.sess.asm_arch.unwrap();

// Default to Intel syntax on x86
Expand Down Expand Up @@ -690,6 +698,22 @@ impl<'gcc, 'tcx> AsmMethods for CodegenCx<'gcc, 'tcx> {
// here unlike normal inline assembly.
template_str.push_str(string);
}

GlobalAsmOperandRef::SymFn { instance } => {
// TODO(@Amanieu): Additional mangling is needed on
// some targets to add a leading underscore (Mach-O)
// or byte count suffixes (x86 Windows).
let name = self.tcx.symbol_name(instance).name;
template_str.push_str(name);
}

GlobalAsmOperandRef::SymStatic { def_id } => {
// TODO(@Amanieu): Additional mangling is needed on
// some targets to add a leading underscore (Mach-O).
let instance = Instance::mono(self.tcx, def_id);
let name = self.tcx.symbol_name(instance).name;
template_str.push_str(name);
}
}
}
}
Expand Down
Loading