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

Jump to bottom

Inline asm #2982

Merged
merged 43 commits into from
Jun 13, 2024
Merged

Inline asm #2982

merged 43 commits into from
Jun 13, 2024

Conversation

badumbatish
Copy link
Contributor

Draft PR for my mentors

@badumbatish badumbatish force-pushed the inline_asm branch 6 times, most recently from 5a0f0a9 to 69a06ce Compare May 22, 2024 04:28
CohenArthur
CohenArthur reviewed May 24, 2024
View reviewed changes
gcc/rust/ast/rust-expr.h Outdated Show resolved Hide resolved
gcc/rust/ast/rust-expr.h Outdated Show resolved Hide resolved
gcc/rust/expand/rust-macro-builtins.cc Outdated Show resolved Hide resolved
@badumbatish badumbatish force-pushed the inline_asm branch 5 times, most recently from ec44fbd to fedb5ed Compare May 30, 2024 01:35
CohenArthur
CohenArthur approved these changes May 30, 2024
View reviewed changes
Copy link
Member

@CohenArthur CohenArthur left a comment

Choose a reason for hiding this comment

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

this is great progress :DD well done! a couple nitpicks and things to fix but we can merge this soon

gcc/rust/ast/rust-expr.h Show resolved Hide resolved
gcc/rust/ast/rust-expr.h Outdated Show resolved Hide resolved
gcc/rust/ast/rust-expr.h Outdated Show resolved Hide resolved
gcc/rust/ast/rust-expr.h Outdated Show resolved Hide resolved
gcc/rust/ast/rust-expr.h Outdated Show resolved Hide resolved
gcc/rust/expand/rust-macro-builtins-asm.cc Outdated Show resolved Hide resolved
gcc/rust/expand/rust-macro-builtins-asm.cc Outdated Show resolved Hide resolved
gcc/rust/expand/rust-macro-builtins-asm.cc Outdated Show resolved Hide resolved
gcc/rust/expand/rust-macro-builtins-asm.cc Outdated Show resolved Hide resolved
gcc/rust/expand/rust-macro-builtins.cc Outdated Show resolved Hide resolved
@badumbatish
Copy link
Contributor Author

Pr's looking good, i let some quoted strings warnings slipped through but it should get fixed soon. I'll look into tl::expected soon

@badumbatish
Almost done with top level parsing
4ba9f94 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (struct AsmParseError): title.
	(enum InlineAsmDirSpec): title.
	(enum InlineAsmOptions): title.
	(struct AsmArg): title.
	(parseAsmArg): title.
	(parse_global_asm): title.
	(parse_nonglobal_asm): title.
	(parse_asm): title.
	(parseDirSpec): title.
	(parse_format_string): title.
	(MacroBuiltin::global_asm_handler): title.
	(MacroBuiltin::nonglobal_asm_handler): title.
	* expand/rust-macro-builtins.cc: title.
	* expand/rust-macro-builtins.h: title.
@badumbatish
Working on parseAsmArg
b7e8168 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (enum InlineAsmRegOrRegClass): title.
	(parseAsmArg): title.
	(check_identifier): title.
	(parse_operand): title.
	(parse_options): title.
	(parse_reg): title.
	(parseDirSpec): title.
	(parse_asm): title.
@badumbatish
Introduce first implementation of parse_clobber_abi
028d109 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_clobber_abi): title.
	(parseAsmArg): title.
	* expand/rust-macro-builtins-asm.h (parse_clobber_abi): title.
@badumbatish
Working on parse_options for a bit more
d47662e 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_options): title.
@badumbatish
Fix clobber_api brackets
e04e33e 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_clobber_abi): title.
@badumbatish
First draft of parse_option finished
801d334 
Finish up on parse_option, formatted parse_clobber_abi
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_clobber_abi): format
	(check_and_set): helper function, is try_set_option equivalent
	(parse_options): new function
	* expand/rust-macro-builtins-asm.h (enum InlineAsmOptions):
	removed
	(check_and_set): decl of helper function
@badumbatish
Introduced is_global_asm to InlineAsm AST
16e36e1 
gcc/rust/ChangeLog:

	* ast/rust-expr.h: Introduced is_global_asm to InlineAsm AST
@badumbatish
Make InlineAsm non-abstract for usage in parsing.
f89f8b1 
gcc/rust/ChangeLog:

	* ast/rust-expr.h: Make InlineAsm non-abstract for usage in parsing.
@badumbatish
Replace scaffolded InlineAsm with real InlineAsm.
d662c59 
Replace scaffolded InlineAsm with real InlineAsm node in rust-expr.h

gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parseDirSpec): replace
	scaffolded InlineAsm with real InlineAsm.
	(parse_clobber_abi): likewise.
	(check_and_set): likewise.
	(parse_options): likewise.
	(parseAsmArg): likewise.
	(parse_asm): likewise.
	* expand/rust-macro-builtins-asm.h (struct AsmParseError): likewise.
	(enum InlineAsmDirSpec): likewise.
	(enum InlineAsmRegOrRegClass): likewise.
	(struct AsmArg): likewise.
	(parseAsmArg): likewise.
	(check_and_set): likewise.
	(parse_operand): likewise.
	(parse_options): likewise.
	(parse_reg): likewise.
	(parse_clobber_abi): likewise.
@badumbatish
Add a simple no-op test for asm!
3efa288 
gcc/testsuite/ChangeLog:

	* rust/compile/inline_asm_nop.rs: Simple test for asm!
@badumbatish
Top level parsing test for asm!
0e7a087 
gcc/testsuite/ChangeLog:

	* rust/compile/inline_asm_ident_first.rs: New test.
@badumbatish
Add tests parsing with trailing commas inline asm
a547765 
gcc/testsuite/ChangeLog:

	* rust/compile/inline_asm_nop_2.rs: New test.
@badumbatish
Added faulty tests for inline asm cloberring
2e7f535 
gcc/testsuite/ChangeLog:

	* rust/compile/inline_asm_faulty_clobber.rs: New test.
	* rust/compile/inline_asm_faulty_clobber_1.rs: New test.
	* rust/compile/inline_asm_faulty_clobber_2.rs: New test.
@badumbatish
Implemented parse_clobber_abi to pass new tests
610dd55 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_clobber_abi):
	implemented parse_clobber_abi
	(parse_format_string): likewise.
	(parseAsmArg): likewise.
	(parse_asm): likewise.
	* expand/rust-macro-builtins-asm.h (parseAsmArg): likewise.
@badumbatish
Parsing of options(...) done.
9ee9c9a 
This is without any mutually exclusive options checked, or
any relationship with reg_operands. Very primitive.

gcc/rust/ChangeLog:

	* ast/rust-expr.h: parsing of options(...)
	* expand/rust-macro-builtins-asm.cc (check_and_set):
	likewise.
	(parse_options): likewise.
	(parseAsmArg): likewise.
	* expand/rust-macro-builtins-asm.h (check_and_set):
	likewise.

gcc/testsuite/ChangeLog:

	* rust/compile/inline_asm_legal_options.rs: New test.
@badumbatish
Add support for AST to HIR inline asm translation
b1a7a2e 
gcc/rust/ChangeLog:

	* ast/rust-expr.h:
	Add support for AST to HIR inline asm translation
	* hir/rust-ast-lower-base.cc (ASTLoweringBase::visit): Likewise.
	* hir/rust-ast-lower-base.h: Likewise.
	* hir/rust-ast-lower-expr.cc (ASTLoweringExpr::visit): Likewise.
	* hir/rust-ast-lower-expr.h: Likewise.
	* hir/tree/rust-hir-expr.h (class InlineAsm): Likewise.
@badumbatish
Partial unsafe support for inline asm
6bd98af 
gcc/rust/ChangeLog:

	* checks/errors/rust-unsafe-checker.cc (UnsafeChecker::visit):
	Partial unsafe support for inline asm
	* checks/errors/rust-unsafe-checker.h: Likewise.
	* hir/tree/rust-hir-expr.h: Likewise.
	* hir/tree/rust-hir.cc (InlineAsm::accept_vis): Likewise.
@badumbatish
Fix warnings
6b83062 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_reg_operand):
	Fix compile warnings.
	(parse_options): Likewise.
	(parse_asm): Likewise.

gcc/testsuite/ChangeLog:

	* rust/compile/inline_asm_illegal_options.rs:
@badumbatish
Scaffolding ast visitor for InlineAsm
4936739 
gcc/rust/ChangeLog:

	* ast/rust-ast-visitor.h:
	Scaffolding ast visitor for InlineAsm
	* ast/rust-ast.cc (InlineAsm::accept_vis): Likewise.
	* ast/rust-expr.h: Likewise.
@badumbatish
Scaffolding HIRFullVisitor for inline asm
87f3663 
gcc/rust/ChangeLog:

	* ast/rust-ast-visitor.h:
	Scaffolding HIRFullVisitor for inline asm
	* ast/rust-ast.cc (InlineAsm::accept_vis): Likewise.
	* hir/tree/rust-hir-visitor.h (RUST_HIR_VISITOR_H): Likewise.
	* hir/tree/rust-hir.cc (InlineAsm::accept_vis): Likewise.
@badumbatish
Successful parse of in and inout, albeit with str
36444b5 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_reg_operand):
	Successful parse of in and inout, albeit with str
	(check_identifier): Likewise.
	(parse_asm_arg): Likewise.
	* expand/rust-macro-builtins-asm.h (parse_format_string): Likewise.

gcc/testsuite/ChangeLog:

	* rust/compile/inline_asm_parse_operand.rs: New test.
@badumbatish
Add potentially_nonpromoted_keywords set str
a2e6d80 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_reg_operand):
	Add potentially_nonpromoted_keywords set str
	(check_identifier): likewise
	* expand/rust-macro-builtins-asm.h (parse_format_string):
	likewise

gcc/testsuite/ChangeLog:

	* rust/compile/inline_asm_parse_operand.rs: fix warnings
@badumbatish
Fix visitor-related warnings
5d2759e 
gcc/rust/ChangeLog:

	* ast/rust-ast-collector.cc (TokenCollector::visit):
	Fix visitor-related warnings
	* ast/rust-ast-collector.h: Likewise.
	* ast/rust-ast-visitor.cc (DefaultASTVisitor::visit): Likewise.
	* ast/rust-ast-visitor.h: Likewise.
	* checks/errors/borrowck/rust-bir-builder-struct.h: Likewise.
	* checks/errors/borrowck/rust-function-collector.h: Likewise.
	* checks/errors/rust-const-checker.cc (ConstChecker::visit): Likewise.
	* checks/errors/rust-const-checker.h: Likewise.
	* expand/rust-derive.h: Likewise.
	* expand/rust-macro-builtins-asm.cc (parse_reg_operand): Likewise.
	* hir/rust-hir-dump.cc (Dump::visit): Likewise.
	* hir/rust-hir-dump.h: Likewise.
	* hir/tree/rust-hir-visitor.h: Likewise.
	* resolve/rust-ast-resolve-base.cc (ResolverBase::visit): Likewise.
	* resolve/rust-ast-resolve-base.h: Likewise.
@badumbatish
Refactoring and supporting more parse_reg_operand
3a16974 
gcc/rust/ChangeLog:

	* ast/rust-expr.h (struct InlineAsmOperand):
	Refactoring and supporting more parse_reg_operand
	* expand/rust-macro-builtins-asm.cc (parse_reg_operand):
	Likewise.
	(rust_debug): Likewise.
@badumbatish
Partial support for operand
2f513e9 
gcc/rust/ChangeLog:

	* ast/rust-expr.h (struct InlineAsmOperand):
	Partial support for operand
	* expand/rust-macro-builtins-asm.cc (parse_reg_operand): Likewise.
	(parse_label): Likewise.
	* expand/rust-macro-builtins-asm.h (parse_label): Likewise.
@badumbatish
Scaffolding validation of asm!
06045d5 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_asm):
	Scaffolding validation of asm!
	(validate): Likewise
	* expand/rust-macro-builtins-asm.h (validate): Likewise
@badumbatish
Update parser to parse strings in the first stage
4fd8909 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_reg_operand):
	Update parser to parse strings in the first stage
	(parse_label): not needed right now
@badumbatish
Move parser and last_token_id to InlineAsmCtx
f928102 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_clobber_abi):
	Move parser and last_token_id to InlineAsmCtx to prepapre
	for tl::expected.
	(parse_reg): Likewise.
	(parse_operand): Likewise.
	(parse_reg_operand): Likewise.
	(check_and_set): Likewise.
	(parse_options): Likewise.
	(parse_format_string): Likewise.
	(parse_asm_arg): Likewise.
	(parse_asm): Likewise.
	* expand/rust-macro-builtins-asm.h (class InlineAsmParseError): Likewise.
	(parse_asm_arg): Likewise.
	(check_and_set): Likewise.
	(parse_operand): Likewise.
	(parse_reg_operand): Likewise.
	(parse_options): Likewise.
	(parse_reg): Likewise.
	(parse_clobber_abi): Likewise.
	(parse_format_string): Likewise.
@badumbatish
Expected first layer done
ede73e2 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parse_reg):
	Expected first layer done
	(parse_reg_operand): Likewise.
	(parse_asm_arg): Likewise.
	(parse_format_strings): Likewise.
	(parse_asm): Likewise.
	(validate): Likewise.
	* expand/rust-macro-builtins-asm.h (parse_asm_arg): Likewise.
	(validate): Likewise.
	(parse_format_strings): Likewise.
@badumbatish
Partial second layer of expected in parsing asm
75c78b4 
gcc/rust/ChangeLog:

	* expand/rust-macro-builtins-asm.cc (parseDirSpec):
	Partial second layer of expected in parsing asm
	(parse_clobber_abi): Likewise
	(parse_operand): Likewise
	(parse_reg_operand): Likewise
	(parse_asm_arg): Likewise
	* expand/rust-macro-builtins-asm.h (parse_clobber_abi): Likewise
	(parse_reg_operand): Likewise
	(parse_operand): Likewise
@badumbatish badumbatish marked this pull request as ready for review June 12, 2024 15:29
CohenArthur
CohenArthur approved these changes Jun 12, 2024
View reviewed changes
gcc/rust/expand/rust-macro-builtins-asm.cc
Comment on lines +34 to +35
tl::expected<InlineAsmContext, std::string>
parse_clobber_abi (InlineAsmContext inline_asm_ctx)
Copy link
Member

Choose a reason for hiding this comment

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

this is good! in another PR, I'd like you to start working on a "proper" error type instead of strings, something like

enum class AsmParseError {
    Reason1,
    Reason2,
    MissingAbiArgument,
    ...
}

it'll clean up the code a little bit and we can then see how we'll transform these into proper Errors to emit for the user

CohenArthur
CohenArthur reviewed Jun 12, 2024
View reviewed changes
gcc/rust/ast/rust-expr.h
@@ -6,6 +6,8 @@
#include "rust-path.h"
#include "rust-macro.h"
#include "rust-operators.h"
#include "rust-system.h"
#include <memory>
Copy link
Member

Choose a reason for hiding this comment

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

ah, the system include is still here. you should be able to remove it entirely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gotcha

gcc/rust/ast/rust-expr.h Show resolved Hide resolved
gcc/rust/ast/rust-expr.h
Comment on lines +4868 to +5072
// TODO: Not sure how outer_attrs plays with InlineAsm, I put it here in order
// to override, very hacky.
std::vector<Attribute> outer_attrs;
Copy link
Member

Choose a reason for hiding this comment

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

ah, alright. can you open an issue for this please? titled something like Handle outer attributes properly for inline assembly and we'll have a look later during the project :)

@CohenArthur CohenArthur added this pull request to the merge queue Jun 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 12, 2024
@CohenArthur CohenArthur added this pull request to the merge queue Jun 13, 2024
This was referenced Jun 13, 2024
Open
Open
Merged via the queue into Rust-GCC:master with commit 7fa14d4 Jun 13, 2024
9 checks passed
@badumbatish badumbatish mentioned this pull request Jun 13, 2024
Open
@badumbatish badumbatish deleted the inline_asm branch June 13, 2024 20:00
@badumbatish badumbatish mentioned this pull request Jun 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CohenArthur CohenArthur CohenArthur approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@badumbatish @CohenArthur