Skip to content

Conversation

@alexander-shaposhnikov
Copy link
Collaborator

This PR adjusts the lexer (following the surrounding style) to make sure the comments are consumed
exactly after CommentString.
This enables to feed the output of clang on aarch64 into llvm-mca (previously broken - the comments
//LLVM-MCA-BEGIN and //LLVM-MCA-END were actually ignored (see e.g. https://godbolt.org/z/PEzW7Tvra)).

Test plan: ninja check-all

@llvmbot llvmbot added the llvm:mc Machine (object) code label Jul 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 7, 2025

@llvm/pr-subscribers-mc

Author: Alexander Shaposhnikov (alexander-shaposhnikov)

Changes

This PR adjusts the lexer (following the surrounding style) to make sure the comments are consumed
exactly after CommentString.
This enables to feed the output of clang on aarch64 into llvm-mca (previously broken - the comments
//LLVM-MCA-BEGIN and //LLVM-MCA-END were actually ignored (see e.g. https://godbolt.org/z/PEzW7Tvra)).

Test plan: ninja check-all


Full diff: https://github.com/llvm/llvm-project/pull/147228.diff

3 Files Affected:

  • (modified) llvm/lib/MC/MCParser/AsmLexer.cpp (+3-1)
  • (added) llvm/test/MC/AsmParser/preserve-comments-aarch64-linux.s (+10)
  • (added) llvm/test/tools/llvm-mca/AArch64/Neoverse/llvm-mca-markers.s (+36)
diff --git a/llvm/lib/MC/MCParser/AsmLexer.cpp b/llvm/lib/MC/MCParser/AsmLexer.cpp
index 3db9ed3199dd8..968ccf776440b 100644
--- a/llvm/lib/MC/MCParser/AsmLexer.cpp
+++ b/llvm/lib/MC/MCParser/AsmLexer.cpp
@@ -834,8 +834,10 @@ AsmToken AsmLexer::LexToken() {
       return LexLineComment();
   }
 
-  if (isAtStartOfComment(TokStart))
+  if (isAtStartOfComment(TokStart)) {
+    CurPtr += MAI.getCommentString().size() - 1;
     return LexLineComment();
+  }
 
   if (isAtStatementSeparator(TokStart)) {
     CurPtr += strlen(MAI.getSeparatorString()) - 1;
diff --git a/llvm/test/MC/AsmParser/preserve-comments-aarch64-linux.s b/llvm/test/MC/AsmParser/preserve-comments-aarch64-linux.s
new file mode 100644
index 0000000000000..acaa90b012008
--- /dev/null
+++ b/llvm/test/MC/AsmParser/preserve-comments-aarch64-linux.s
@@ -0,0 +1,10 @@
+	// REQUIRES: aarch64-registered-target
+	// RUN: llvm-mc -preserve-comments -n -triple aarch64-unknown-linux-gnu < %s > %t
+	// RUN: diff -b %s %t
+
+	.text
+
+foo:
+	// comment here
+	nop
+	// comment here too
diff --git a/llvm/test/tools/llvm-mca/AArch64/Neoverse/llvm-mca-markers.s b/llvm/test/tools/llvm-mca/AArch64/Neoverse/llvm-mca-markers.s
new file mode 100644
index 0000000000000..d5a291b1d7102
--- /dev/null
+++ b/llvm/test/tools/llvm-mca/AArch64/Neoverse/llvm-mca-markers.s
@@ -0,0 +1,36 @@
+# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
+// RUN: llvm-mca -mtriple=aarch64-unknown-linux-gnu -mcpu=neoverse-v2 -iterations=1 -resource-pressure=false < %s | FileCheck %s
+
+.text
+// LLVM-MCA-BEGIN Empty
+// Empty sequence
+// LLVM-MCA-END
+
+  mul x1, x1, x1
+// LLVM-MCA-BEGIN NotEmpty
+  add x0, x0, x1
+// LLVM-MCA-END
+  mul x2, x2, x2
+
+# CHECK:      [0] Code Region - NotEmpty
+
+# CHECK:      Iterations:        1
+# CHECK-NEXT: Instructions:      1
+# CHECK-NEXT: Total Cycles:      4
+# CHECK-NEXT: Total uOps:        1
+
+# CHECK:      Dispatch Width:    6
+# CHECK-NEXT: uOps Per Cycle:    0.25
+# CHECK-NEXT: IPC:               0.25
+# CHECK-NEXT: Block RThroughput: 0.2
+
+# CHECK:      Instruction Info:
+# CHECK-NEXT: [1]: #uOps
+# CHECK-NEXT: [2]: Latency
+# CHECK-NEXT: [3]: RThroughput
+# CHECK-NEXT: [4]: MayLoad
+# CHECK-NEXT: [5]: MayStore
+# CHECK-NEXT: [6]: HasSideEffects (U)
+
+# CHECK:      [1]    [2]    [3]    [4]    [5]    [6]    Instructions:
+# CHECK-NEXT:  1      1     0.17                        add	x0, x0, x1

@alexander-shaposhnikov
Copy link
Collaborator Author

no, llvm-mc still prints tabs (with -o), i think that's the reason why some other tests contain them as well

@alexander-shaposhnikov alexander-shaposhnikov merged commit 4946db1 into llvm:main Jul 7, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:mc Machine (object) code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants