[ms] [llvm-ml] Allow optional parenthesized arguments for macros #129905
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We match ML64.EXE, which allows optional parentheses around a macro's arguments.
|
@llvm/pr-subscribers-mc Author: Eric Astor (ericastor) ChangesWe match ML64.EXE, which allows optional parentheses around a macro's arguments. Full diff: https://github.com/llvm/llvm-project/pull/129905.diff 3 Files Affected:
diff --git a/llvm/lib/MC/MCParser/AsmLexer.cpp b/llvm/lib/MC/MCParser/AsmLexer.cpp
index 41ff94d125a16..23836438027c0 100644
--- a/llvm/lib/MC/MCParser/AsmLexer.cpp
+++ b/llvm/lib/MC/MCParser/AsmLexer.cpp
@@ -698,8 +698,10 @@ size_t AsmLexer::peekTokens(MutableArrayRef<AsmToken> Buf,
Buf[ReadCount] = Token;
- if (Token.is(AsmToken::Eof))
+ if (Token.is(AsmToken::Eof)) {
+ ReadCount++;
break;
+ }
}
SetError(SavedErrLoc, SavedErr);
diff --git a/llvm/lib/MC/MCParser/MasmParser.cpp b/llvm/lib/MC/MCParser/MasmParser.cpp
index 4ef781c54f11d..15452c8c0822e 100644
--- a/llvm/lib/MC/MCParser/MasmParser.cpp
+++ b/llvm/lib/MC/MCParser/MasmParser.cpp
@@ -1986,7 +1986,10 @@ bool MasmParser::parseStatement(ParseStatementInfo &Info,
// If macros are enabled, check to see if this is a macro instantiation.
if (const MCAsmMacro *M = getContext().lookupMacro(IDVal.lower())) {
- return handleMacroEntry(M, IDLoc);
+ AsmToken::TokenKind ArgumentEndTok = parseOptionalToken(AsmToken::LParen)
+ ? AsmToken::RParen
+ : AsmToken::EndOfStatement;
+ return handleMacroEntry(M, IDLoc, ArgumentEndTok);
}
// Otherwise, we have a normal instruction or directive.
@@ -2830,7 +2833,7 @@ bool MasmParser::handleMacroEntry(const MCAsmMacro *M, SMLoc NameLoc,
}
MCAsmMacroArguments A;
- if (parseMacroArguments(M, A, ArgumentEndTok))
+ if (parseMacroArguments(M, A, ArgumentEndTok) || parseToken(ArgumentEndTok))
return true;
// Macro instantiation is lexical, unfortunately. We construct a new buffer
@@ -2914,10 +2917,6 @@ bool MasmParser::handleMacroInvocation(const MCAsmMacro *M, SMLoc NameLoc) {
eatToEndOfStatement();
}
- // Consume the right-parenthesis on the other side of the arguments.
- if (parseRParen())
- return true;
-
// Exit values may require lexing, unfortunately. We construct a new buffer to
// hold the exit value.
std::unique_ptr<MemoryBuffer> MacroValue =
diff --git a/llvm/test/tools/llvm-ml/macro.asm b/llvm/test/tools/llvm-ml/macro.asm
index 3c81b89414fc7..3373ff4837f7c 100644
--- a/llvm/test/tools/llvm-ml/macro.asm
+++ b/llvm/test/tools/llvm-ml/macro.asm
@@ -100,6 +100,25 @@ substitution_test_uppercase PROC
ret
substitution_test_uppercase ENDP
+substitution_test_with_parentheses PROC
+; CHECK-LABEL: substitution_test_with_parentheses:
+
+ SubstitutionMacro(2, 8)
+; CHECK: mov eax, 2
+; CHECK-NEXT: mov eax, 2
+; CHECK-NEXT: mov eax, 2
+; CHECK-NEXT: mov eax, 2
+; CHECK: mov eax, dword ptr [rip + xa1]
+; CHECK-NEXT: mov eax, dword ptr [rip + x2]
+; CHECK-NEXT: mov eax, dword ptr [rip + x2]
+; CHECK: mov eax, 8
+; CHECK-NEXT: mov eax, 8
+; CHECK-NEXT: mov eax, 8
+; CHECK-NEXT: mov eax, 8
+
+ ret
+substitution_test_with_parentheses ENDP
+
AmbiguousSubstitutionMacro MACRO x, y
x&y BYTE 0
ENDM
|
ericastor
requested review from
MaskRay and
mstorsjo
and removed request for
MaskRay
mstorsjo
approved these changes
Mar 11, 2025
| @@ -698,8 +698,10 @@ size_t AsmLexer::peekTokens(MutableArrayRef<AsmToken> Buf, | |||
|
|
|||
| Buf[ReadCount] = Token; | |||
|
|
|||
| if (Token.is(AsmToken::Eof)) | |||
| if (Token.is(AsmToken::Eof)) { | |||
| ReadCount++; | |||
There was a problem hiding this comment.
Is this change related to the rest, or is it a separate drive-by fix?
There was a problem hiding this comment.
... it's been a while since I wrote this, and I'm sorry to say I'm not sure. I have a vague memory that it was related in a surprising way.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/1071 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We match ML64.EXE, which allows optional parentheses around a macro's arguments.