Hi Andrew, On 4/28/25 6:05 PM, Andrew Pinski wrote:
On Mon, Apr 28, 2025 at 8:48 AM <arthur.co...@embecosm.com> wrote:From: Pierre-Emmanuel Patry <pierre-emmanuel.pa...@embecosm.com> InlineAsm node does not support memory clobbers.A few review on this. I think this should just be called extended rather than referencing LLVM here.
This node exists solely for handling the `llvm_asm!()` built-in macro in the Rust language. It is an old macro, still in use in Rust 1.49 as inline assembly was not completely stabilized yet, but has been completely removed and reworked in recent versions of the language.
The reason we need to add this node is to handle a very specific use case of inline assembly, the `black_box` hint function (which was added as a test case in a later patch, also in this patchset).
pub fn black_box<T>(mut dummy: T) -> T {
// We need to "use" the argument in some way LLVM can't introspect,
and on
// targets that support it we can typically leverage inline
assembly to do
// this. LLVM's interpretation of inline assembly is that it's,
well, a black
// box. This isn't the greatest implementation since it probably
deoptimizes
// more than we want, but it's so far good enough.
#[cfg(not(miri))] // This is just a hint, so it is fine to skip in
Miri.
// SAFETY: the inline assembly is a no-op.
unsafe {
// FIXME: Cannot use `asm!` because it doesn't support MIPS and
other architectures.
llvm_asm!("" : : "r"(&mut dummy) : "memory" : "volatile");
}
dummy
}
In recent versions of the language, this function became an intrinsic.
So as we catch up to a more modern Rust, we will be able to remove this
node entirely and just handle Rust's version of inline assembly.
Regardless, we are targetting Rust 1.49 and must thus handle this one specific macro invocation of "LLVM assembly", which is why we added this node. The parsing rules are a bit different than "regular" Rust inline assembly, which is why Pierre-Emmanuel prefered to add a new node.
Apart from a few extremely specific use cases, it is not going to be used and will be removed as soon as possible.
gcc/rust/ChangeLog: * ast/rust-ast-collector.cc (TokenCollector::visit): Make visitor unreachable. * ast/rust-ast-collector.h: Add visit for LlvmInlineAsmNode. * ast/rust-ast-visitor.cc (DefaultASTVisitor::visit): Add visit function for the default ast visitor. * ast/rust-ast-visitor.h: Add function prototype. * ast/rust-ast.cc (LlvmInlineAsm::accept_vis): Add accept_vis to LlvmInlineAsm node. * ast/rust-ast.h: Add LlvmInlineAsm node kind. * ast/rust-expr.h (class LlvmInlineAsm): Add LlvmInlineAsm node. * expand/rust-derive.h: Add visit function for LlvmInlineAsm node. * expand/rust-macro-builtins-asm.cc (MacroBuiltin::llvm_asm_handler): Add handler for llvm inline assembly nodes. (parse_llvm_asm): Add function to parse llvm assembly nodes. * expand/rust-macro-builtins-asm.h (parse_llvm_asm): Add function prototypes. * expand/rust-macro-builtins.cc (inline_llvm_asm_maker): Add macro transcriber. * expand/rust-macro-builtins.h: Add transcriber function prototype. * hir/rust-ast-lower-base.cc (ASTLoweringBase::visit): Add visit function for LlvmInlineAsm node. * hir/rust-ast-lower-base.h: Add visit function prototype. * resolve/rust-ast-resolve-base.cc (ResolverBase::visit): Add visit function for LlvmInlineAsm node. * resolve/rust-ast-resolve-base.h: Add visit function prototype. Signed-off-by: Pierre-Emmanuel Patry <pierre-emmanuel.pa...@embecosm.com> --- gcc/rust/ast/rust-ast-collector.cc | 10 ++++- gcc/rust/ast/rust-ast-collector.h | 1 + gcc/rust/ast/rust-ast-visitor.cc | 4 ++ gcc/rust/ast/rust-ast-visitor.h | 2 + gcc/rust/ast/rust-ast.cc | 6 +++ gcc/rust/ast/rust-ast.h | 1 + gcc/rust/ast/rust-expr.h | 48 ++++++++++++++++++++++ gcc/rust/expand/rust-derive.h | 1 + gcc/rust/expand/rust-macro-builtins-asm.cc | 21 ++++++++++ gcc/rust/expand/rust-macro-builtins-asm.h | 7 ++++ gcc/rust/expand/rust-macro-builtins.cc | 11 ++++- gcc/rust/expand/rust-macro-builtins.h | 4 ++ gcc/rust/hir/rust-ast-lower-base.cc | 4 ++ gcc/rust/hir/rust-ast-lower-base.h | 1 + gcc/rust/resolve/rust-ast-resolve-base.cc | 4 ++ gcc/rust/resolve/rust-ast-resolve-base.h | 1 + 16 files changed, 124 insertions(+), 2 deletions(-) diff --git a/gcc/rust/ast/rust-ast-collector.cc b/gcc/rust/ast/rust-ast-collector.cc index 8ee63752f32..90ff2e36251 100644 --- a/gcc/rust/ast/rust-ast-collector.cc +++ b/gcc/rust/ast/rust-ast-collector.cc @@ -1520,7 +1520,15 @@ TokenCollector::visit (AsyncBlockExpr &expr) void TokenCollector::visit (InlineAsm &expr) -{} +{ + rust_unreachable (); +} + +void +TokenCollector::visit (LlvmInlineAsm &expr) +{ + rust_unreachable (); +} // rust-item.h diff --git a/gcc/rust/ast/rust-ast-collector.h b/gcc/rust/ast/rust-ast-collector.h index b014c23ed0b..f45e3cc51ae 100644 --- a/gcc/rust/ast/rust-ast-collector.h +++ b/gcc/rust/ast/rust-ast-collector.h @@ -303,6 +303,7 @@ public: void visit (AwaitExpr &expr); void visit (AsyncBlockExpr &expr); void visit (InlineAsm &expr); + void visit (LlvmInlineAsm &expr); // rust-item.h void visit (TypeParam ¶m); void visit (LifetimeWhereClauseItem &item); diff --git a/gcc/rust/ast/rust-ast-visitor.cc b/gcc/rust/ast/rust-ast-visitor.cc index 9d524c3cc4c..beb3360c4e1 100644 --- a/gcc/rust/ast/rust-ast-visitor.cc +++ b/gcc/rust/ast/rust-ast-visitor.cc @@ -713,6 +713,10 @@ DefaultASTVisitor::visit (AST::InlineAsm &expr) } } +void +DefaultASTVisitor::visit (AST::LlvmInlineAsm &expr) +{} + void DefaultASTVisitor::visit (AST::TypeParam ¶m) { diff --git a/gcc/rust/ast/rust-ast-visitor.h b/gcc/rust/ast/rust-ast-visitor.h index 51661df76b4..6d243e7b063 100644 --- a/gcc/rust/ast/rust-ast-visitor.h +++ b/gcc/rust/ast/rust-ast-visitor.h @@ -131,6 +131,7 @@ public: virtual void visit (AwaitExpr &expr) = 0; virtual void visit (AsyncBlockExpr &expr) = 0; virtual void visit (InlineAsm &expr) = 0; + virtual void visit (LlvmInlineAsm &expr) = 0; // rust-item.h virtual void visit (TypeParam ¶m) = 0; @@ -314,6 +315,7 @@ public: virtual void visit (AST::AwaitExpr &expr) override; virtual void visit (AST::AsyncBlockExpr &expr) override; virtual void visit (InlineAsm &expr) override; + virtual void visit (LlvmInlineAsm &expr) override; virtual void visit (AST::TypeParam ¶m) override; virtual void visit (AST::LifetimeWhereClauseItem &item) override; diff --git a/gcc/rust/ast/rust-ast.cc b/gcc/rust/ast/rust-ast.cc index 06e0e7b0fbc..4e82be420fe 100644 --- a/gcc/rust/ast/rust-ast.cc +++ b/gcc/rust/ast/rust-ast.cc @@ -4650,6 +4650,12 @@ InlineAsm::accept_vis (ASTVisitor &vis) vis.visit (*this); } +void +LlvmInlineAsm::accept_vis (ASTVisitor &vis) +{ + vis.visit (*this); +} + void TypeParam::accept_vis (ASTVisitor &vis) { diff --git a/gcc/rust/ast/rust-ast.h b/gcc/rust/ast/rust-ast.h index 91611ec6a62..55f178d782c 100644 --- a/gcc/rust/ast/rust-ast.h +++ b/gcc/rust/ast/rust-ast.h @@ -1264,6 +1264,7 @@ public: Await, AsyncBlock, InlineAsm, + LlvmInlineAsm, Identifier, FormatArgs, MacroInvocation, diff --git a/gcc/rust/ast/rust-expr.h b/gcc/rust/ast/rust-expr.h index 69538df63e5..0dfb4e5fd60 100644 --- a/gcc/rust/ast/rust-expr.h +++ b/gcc/rust/ast/rust-expr.h @@ -5330,6 +5330,54 @@ public: Expr::Kind get_expr_kind () const override { return Expr::Kind::InlineAsm; } }; +class LlvmInlineAsm : public ExprWithoutBlock +{ + // llvm_asm!("" : : "r"(&mut dummy) : "memory" : "volatile"); + // Asm, Outputs, Inputs, Clobbers, Options, + +private: + location_t locus; + std::vector<Attribute> outer_attrs; + + std::vector<TupleClobber> clobber_abi; + bool is_volatile; + bool align_stack; + enum class Dialect + { + Att, + Intel, + } dialect;Asm Dialect is target specific so this should just be an integer which gets translated via target hooks instead of a full on enum. Note GCC's extended asm dialects support up a full 32bit #: #ifdef ASSEMBLER_DIALECT /* Number of the assembler dialect to use, starting at 0. */ static int dialect_number; #endif X86 is not the only target which supports an asm dialect: sh, pa, pdp11 and bpf are other targets which support one. Thanks, Andew Pinski
The enum is here for parsing, but will never be used in reality. This is just for completeness and for handling the macro properly. In reality, no instructions are supported in the llvm_asm!() macro and it's extremely limited. There are assertions in the parsing logic to prevent non-empty assembly format strings, as well as output operands, etc. So it is extremely limited and we will not be doing anything remotely complex with it, other than handling the `black_box` hint in Rust 1.49.
Best, Arthur
+ +public: + LlvmInlineAsm (location_t locus) : locus (locus) {} + + Dialect get_dialect () { return dialect; } + + location_t get_locus () const override { return locus; } + + void mark_for_strip () override {} + + bool is_marked_for_strip () const override { return false; } + + std::vector<Attribute> &get_outer_attrs () override { return outer_attrs; } + + void accept_vis (ASTVisitor &vis) override; + + std::string as_string () const override { return "InlineAsm AST Node"; } + + void set_outer_attrs (std::vector<Attribute> v) override { outer_attrs = v; } + + LlvmInlineAsm *clone_expr_without_block_impl () const override + { + return new LlvmInlineAsm (*this); + } + + Expr::Kind get_expr_kind () const override + { + return Expr::Kind::LlvmInlineAsm; + } +}; + } // namespace AST } // namespace Rust diff --git a/gcc/rust/expand/rust-derive.h b/gcc/rust/expand/rust-derive.h index d8cc0a480ab..5fca49ca1f3 100644 --- a/gcc/rust/expand/rust-derive.h +++ b/gcc/rust/expand/rust-derive.h @@ -171,6 +171,7 @@ private: virtual void visit (AwaitExpr &expr) override final{}; virtual void visit (AsyncBlockExpr &expr) override final{}; virtual void visit (InlineAsm &expr) override final{}; + virtual void visit (LlvmInlineAsm &expr) override final{}; virtual void visit (TypeParam ¶m) override final{}; virtual void visit (LifetimeWhereClauseItem &item) override final{}; virtual void visit (TypeBoundWhereClauseItem &item) override final{}; diff --git a/gcc/rust/expand/rust-macro-builtins-asm.cc b/gcc/rust/expand/rust-macro-builtins-asm.cc index 97f7e3cff77..8bee4524d49 100644 --- a/gcc/rust/expand/rust-macro-builtins-asm.cc +++ b/gcc/rust/expand/rust-macro-builtins-asm.cc @@ -660,6 +660,15 @@ MacroBuiltin::asm_handler (location_t invoc_locus, AST::MacroInvocData &invoc, return parse_asm (invoc_locus, invoc, semicolon, is_global_asm); } +tl::optional<AST::Fragment> +MacroBuiltin::llvm_asm_handler (location_t invoc_locus, + AST::MacroInvocData &invoc, + AST::InvocKind semicolon, + AST::AsmKind is_global_asm) +{ + return parse_llvm_asm (invoc_locus, invoc, semicolon, is_global_asm); +} + tl::expected<InlineAsmContext, InlineAsmParseError> parse_asm_arg (InlineAsmContext inline_asm_ctx) { @@ -970,4 +979,16 @@ validate (InlineAsmContext inline_asm_ctx) { return tl::expected<InlineAsmContext, InlineAsmParseError> (inline_asm_ctx); } + +tl::optional<AST::Fragment> +parse_llvm_asm (location_t invoc_locus, AST::MacroInvocData &invoc, + AST::InvocKind semicolon, AST::AsmKind is_global_asm) +{ + MacroInvocLexer lex (invoc.get_delim_tok_tree ().to_token_stream ()); + Parser<MacroInvocLexer> parser (lex); + auto last_token_id = macro_end_token (invoc.get_delim_tok_tree (), parser); + + AST::LlvmInlineAsm llvm_asm{invoc_locus}; +} + } // namespace Rust diff --git a/gcc/rust/expand/rust-macro-builtins-asm.h b/gcc/rust/expand/rust-macro-builtins-asm.h index 8081dae5140..e13ee25a935 100644 --- a/gcc/rust/expand/rust-macro-builtins-asm.h +++ b/gcc/rust/expand/rust-macro-builtins-asm.h @@ -172,4 +172,11 @@ tl::optional<std::string> parse_label (Parser<MacroInvocLexer> &parser, TokenId last_token_id, InlineAsmContext &inline_asm_ctx); +// LLVM ASM bits + +WARN_UNUSED_RESULT +tl::optional<AST::Fragment> +parse_llvm_asm (location_t invoc_locus, AST::MacroInvocData &invoc, + AST::InvocKind semicolon, AST::AsmKind is_global_asm); + } // namespace Rust diff --git a/gcc/rust/expand/rust-macro-builtins.cc b/gcc/rust/expand/rust-macro-builtins.cc index 8b406fff9e6..b58ed71ba4e 100644 --- a/gcc/rust/expand/rust-macro-builtins.cc +++ b/gcc/rust/expand/rust-macro-builtins.cc @@ -103,6 +103,15 @@ inline_asm_maker (AST::AsmKind global_asm) }; } +AST::MacroTranscriberFunc +inline_llvm_asm_maker (AST::AsmKind global_asm) +{ + return [global_asm] (location_t loc, AST::MacroInvocData &invoc, + AST::InvocKind semicolon) { + return MacroBuiltin::llvm_asm_handler (loc, invoc, semicolon, global_asm); + }; +} + std::unordered_map<std::string, AST::MacroTranscriberFunc> MacroBuiltin::builtin_transcribers = { {"assert", MacroBuiltin::assert_handler}, @@ -121,7 +130,7 @@ std::unordered_map<std::string, AST::MacroTranscriberFunc> {"format_args_nl", format_args_maker (AST::FormatArgs::Newline::Yes)}, {"asm", inline_asm_maker (AST::AsmKind::Inline)}, // FIXME: Is that okay? - {"llvm_asm", inline_asm_maker (AST::AsmKind::Inline)}, + {"llvm_asm", inline_llvm_asm_maker (AST::AsmKind::Inline)}, {"global_asm", inline_asm_maker (AST::AsmKind::Global)}, {"option_env", MacroBuiltin::option_env_handler}, /* Unimplemented macro builtins */ diff --git a/gcc/rust/expand/rust-macro-builtins.h b/gcc/rust/expand/rust-macro-builtins.h index ff06ebf2289..541e95636b1 100644 --- a/gcc/rust/expand/rust-macro-builtins.h +++ b/gcc/rust/expand/rust-macro-builtins.h @@ -180,6 +180,10 @@ public: AST::InvocKind semicolon, AST::AsmKind is_global_asm); + static tl::optional<AST::Fragment> + llvm_asm_handler (location_t invoc_locus, AST::MacroInvocData &invoc, + AST::InvocKind semicolon, AST::AsmKind is_global_asm); + static tl::optional<AST::Fragment> format_args_handler (location_t invoc_locus, AST::MacroInvocData &invoc, AST::InvocKind semicolon, AST::FormatArgs::Newline nl); diff --git a/gcc/rust/hir/rust-ast-lower-base.cc b/gcc/rust/hir/rust-ast-lower-base.cc index 5039798a810..2d9a4450c90 100644 --- a/gcc/rust/hir/rust-ast-lower-base.cc +++ b/gcc/rust/hir/rust-ast-lower-base.cc @@ -267,6 +267,10 @@ void ASTLoweringBase::visit (AST::InlineAsm &) {} +void +ASTLoweringBase::visit (AST::LlvmInlineAsm &) +{} + // void ASTLoweringBase::visit(MatchCasematch_case) {} // void ASTLoweringBase:: (AST::MatchCaseBlockExpr &) {} // void ASTLoweringBase:: (AST::MatchCaseExpr &) {} diff --git a/gcc/rust/hir/rust-ast-lower-base.h b/gcc/rust/hir/rust-ast-lower-base.h index b3bb174babf..31161810c6f 100644 --- a/gcc/rust/hir/rust-ast-lower-base.h +++ b/gcc/rust/hir/rust-ast-lower-base.h @@ -152,6 +152,7 @@ public: virtual void visit (AST::IfLetExpr &expr) override; virtual void visit (AST::IfLetExprConseqElse &expr) override; virtual void visit (AST::InlineAsm &expr) override; + virtual void visit (AST::LlvmInlineAsm &expr) override; // virtual void visit(MatchCase& match_case) override; // virtual void visit (AST::MatchCaseBlockExpr &match_case) override; // virtual void visit (AST::MatchCaseExpr &match_case) override; diff --git a/gcc/rust/resolve/rust-ast-resolve-base.cc b/gcc/rust/resolve/rust-ast-resolve-base.cc index 6c35a22b7f6..b781ce33f3f 100644 --- a/gcc/rust/resolve/rust-ast-resolve-base.cc +++ b/gcc/rust/resolve/rust-ast-resolve-base.cc @@ -327,6 +327,10 @@ void ResolverBase::visit (AST::InlineAsm &) {} +void +ResolverBase::visit (AST::LlvmInlineAsm &) +{} + void ResolverBase::visit (AST::TypeParam &) {} diff --git a/gcc/rust/resolve/rust-ast-resolve-base.h b/gcc/rust/resolve/rust-ast-resolve-base.h index ab74e84f079..5bb9e4f1822 100644 --- a/gcc/rust/resolve/rust-ast-resolve-base.h +++ b/gcc/rust/resolve/rust-ast-resolve-base.h @@ -110,6 +110,7 @@ public: void visit (AST::AwaitExpr &); void visit (AST::AsyncBlockExpr &); void visit (AST::InlineAsm &); + void visit (AST::LlvmInlineAsm &); void visit (AST::TypeParam &); -- 2.49.0
OpenPGP_0x1B3465B044AD9C65.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
