On Mon, Apr 28, 2025 at 9:05 AM Andrew Pinski <pins...@gmail.com> 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.
>
> >
> > 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.
I forgot to mention, GCC does not currently have a way to change the
dialect dynamically. it is controlled by the backend only. Do you have
patches to final.cc and the backends to support dynamically changing
the dialect while processing inline-asm?
Thanks,
Andrew Pinski
>
> Thanks,
> Andew Pinski
>
> > +
> > +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
> >
