Treat ';' and '\n' as assembly instruction separators in

collectAsmInstrs

This also fixes the incorrect treatment of '\n\t' as a separator for asm
instructions.

Author: rlavaee

llvm/include/llvm/IR/InlineAsm.h

@@ -87,7 +87,12 @@ class InlineAsm final : public Value {
 
   StringRef getAsmString() const { return AsmString; }
   StringRef getConstraintString() const { return Constraints; }
-  LLVM_ABI void collectAsmStrs(SmallVectorImpl<StringRef> &AsmStrs) const;
+
+  /// collectAsmInstrs - Parses the assembly instruction and collects individual
+  /// instructions in a vector. Handles both '\n' and ';' as instruction
+  /// separators. Trims comments (marked by '#' and "//") and whitespaces from
+  /// instructions.
+  LLVM_ABI SmallVector<StringRef> collectAsmInstrs() const;
 
   /// This static method can be used by the parser to check to see if the
   /// specified constraint string is legal for the type.

llvm/lib/Analysis/InlineCost.cpp

@@ -793,33 +793,24 @@ class InlineCostCallAnalyzer final : public CallAnalyzer {
   void onInlineAsm(const InlineAsm &Arg) override {
     if (!InlineAsmInstrCost)
       return;
-    SmallVector<StringRef, 4> AsmStrs;
-    Arg.collectAsmStrs(AsmStrs);
     int SectionLevel = 0;
     int InlineAsmInstrCount = 0;
-    for (StringRef AsmStr : AsmStrs) {
-      // Trim whitespaces and comments.
-      StringRef Trimmed = AsmStr.trim();
-      size_t hashPos = Trimmed.find('#');
-      if (hashPos != StringRef::npos)
-        Trimmed = Trimmed.substr(0, hashPos);
-      // Ignore comments.
-      if (Trimmed.empty())
-        continue;
+    for (StringRef AsmInstr : Arg.collectAsmInstrs()) {
       // Filter out the outlined assembly instructions from the cost by keeping
       // track of the section level and only accounting for instrutions at
       // section level of zero. Note there will be duplication in outlined
       // sections too, but is not accounted in the inlining cost model.
-      if (Trimmed.starts_with(".pushsection")) {
+      if (AsmInstr.starts_with(".pushsection")) {
         ++SectionLevel;
         continue;
       }
-      if (Trimmed.starts_with(".popsection")) {
+      if (AsmInstr.starts_with(".popsection")) {
         --SectionLevel;
         continue;
       }
-      // Ignore directives and labels.
-      if (Trimmed.starts_with(".") || Trimmed.contains(":"))
+      // Labels are free. Note we only exclude labels that are not followed by
+      // any other instruction.
+      if (AsmInstr.ends_with(":"))
         continue;
       if (SectionLevel == 0)
         ++InlineAsmInstrCount;

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

@@ -9989,8 +9989,7 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
   }
 
   int OpNo = -1;
-  SmallVector<StringRef> AsmStrs;
-  IA->collectAsmStrs(AsmStrs);
+  SmallVector<StringRef> AsmInstrs = IA->collectAsmInstrs();
 
   // Second pass over the constraints: compute which constraint option to use.
   for (SDISelAsmOperandInfo &OpInfo : ConstraintOperands) {
@@ -10034,7 +10033,7 @@ void SelectionDAGBuilder::visitInlineAsm(const CallBase &Call,
     // label, so here we don't handle jmp function label now, but we need to
     // enhance it (especilly in PIC model) if we meet meaningful requirements.
     if (OpInfo.isIndirect && isFunction(OpInfo.CallOperand) &&
-        TLI.isInlineAsmTargetBranch(AsmStrs, OpNo) &&
+        TLI.isInlineAsmTargetBranch(AsmInstrs, OpNo) &&
         TM.getCodeModel() != CodeModel::Large) {
       OpInfo.isIndirect = false;
       OpInfo.ConstraintType = TargetLowering::C_Address;

llvm/lib/IR/InlineAsm.cpp

@@ -60,17 +60,41 @@ FunctionType *InlineAsm::getFunctionType() const {
   return FTy;
 }
 
-void InlineAsm::collectAsmStrs(SmallVectorImpl<StringRef> &AsmStrs) const {
+SmallVector<StringRef> InlineAsm::collectAsmInstrs() const {
+  if (AsmString.empty())
+    return {};
   StringRef AsmStr(AsmString);
-  AsmStrs.clear();
-
-  // TODO: 1) Unify delimiter for inline asm, we also meet other delimiters
-  // for example "\0A", ";".
-  // 2) Enhance StringRef. Some of the special delimiter ("\0") can't be
-  // split in StringRef. Also empty StringRef can not call split (will stuck).
-  if (AsmStr.empty())
-    return;
-  AsmStr.split(AsmStrs, "\n\t", -1, false);
+  // First break the assembly string into lines.
+  SmallVector<StringRef, 4> AsmLines;
+  AsmStr.split(AsmLines, '\n', -1, false);
+
+  auto TrimComments = [&](StringRef &AsmLine) {
+    for (auto CommentMarker : {"#", "//"}) {
+      size_t MarkerPos = AsmLine.find(CommentMarker);
+      if (MarkerPos != StringRef::npos)
+        AsmLine = AsmLine.substr(0, MarkerPos);
+    }
+  };
+
+  SmallVector<StringRef, 4> AsmInstrs;
+  AsmInstrs.reserve(AsmLines.size());
+  for (StringRef &AsmLine : AsmLines) {
+    // First remove the comments. Note it's important to do this before breaking
+    // by ';' since the comment portion may include that character too.
+    TrimComments(AsmLine);
+    if (AsmLine.empty())
+      continue;
+    // Break by ';' to collect separate instructions in a single line.
+    SmallVector<StringRef, 4> CurrentLineAsmInstrs;
+    AsmLine.split(CurrentLineAsmInstrs, ';');
+    for (StringRef S : CurrentLineAsmInstrs) {
+      StringRef Trimmed = S.trim();
+      if (Trimmed.empty())
+        continue;
+      AsmInstrs.push_back(Trimmed);
+    }
+  }
+  return AsmInstrs;
 }
 
 /// Parse - Analyze the specified string (e.g. "==&{eax}") and fill in the

llvm/test/CodeGen/X86/inline-asm-function-call-pic.ll

@@ -59,7 +59,7 @@ define void @func() local_unnamed_addr #0 {
 entry:
   %call = tail call i32 @static_func()
 ;; We test call, CALL, and jmp.
-  tail call void asm sideeffect inteldialect "call ${0:P}\0A\09CALL ${1:P}\0A\09jmp ${1:P}\0A\09shr eax, $$0\0A\09shr ebx, $$0\0A\09shr ecx, $$0\0A\09shr edx, $$0\0A\09shr edi, $$0\0A\09shr esi, $$0\0A\09shr ebp, $$0\0A\09shr esp, $$0", "*m,*m,~{eax},~{ebp},~{ebx},~{ecx},~{edi},~{edx},~{flags},~{esi},~{esp},~{dirflag},~{fpsr},~{flags}"(ptr nonnull elementtype(i32 (...)) @static_func, ptr nonnull elementtype(i32 (...)) @extern_func) #0
+  tail call void asm sideeffect inteldialect "call ${0:P}\0A\09CALL ${1:P}; jmp ${1:P}\0A\09shr eax, $$0\0Ashr ebx, $$0\0A\09shr ecx, $$0\0A\09shr edx, $$0; shr edi, $$0\0A\09shr esi, $$0\0A\09shr ebp, $$0\0A\09shr esp, $$0", "*m,*m,~{eax},~{ebp},~{ebx},~{ecx},~{edi},~{edx},~{flags},~{esi},~{esp},~{dirflag},~{fpsr},~{flags}"(ptr nonnull elementtype(i32 (...)) @static_func, ptr nonnull elementtype(i32 (...)) @extern_func) #0
   ret void
 }
 

llvm/test/CodeGen/X86/inline-asm-p-constraint.ll

@@ -7,10 +7,9 @@ define ptr @foo(ptr %Ptr) {
   %Ptr.addr = alloca ptr, align 8
   store ptr %Ptr, ptr %Ptr.addr, align 8
 ; CHECK: movq    %rdi, -8(%rsp)
-  %1 = tail call ptr asm "mov $1, $0\0A\09lea $2, $0", "=r,p,*m,~{dirflag},~{fpsr},~{flags}"(ptr %Ptr, ptr elementtype(ptr) %Ptr.addr)
+  %1 = tail call ptr asm "mov $1, $0; lea $2, $0", "=r,p,*m,~{dirflag},~{fpsr},~{flags}"(ptr %Ptr, ptr elementtype(ptr) %Ptr.addr)
 ; CHECK-NEXT: #APP
-; CHECK-NEXT: mov (%rdi), %rax
-; CHECK-NEXT: lea -8(%rsp), %rax
+; CHECK-NEXT: mov (%rdi), %rax; lea -8(%rsp), %rax
 ; CHECK-NEXT: #NO_APP
   ret ptr %1
 ; CHECK-NEXT: retq

llvm/test/Transforms/Inline/inline-call-with-asm-call.ll

@@ -27,7 +27,7 @@ define void @caller(i32 %a, i1 %b) #0 {
 ;; destination section and two assembly instructions in the outlined "other"
 ;; section.
 define void @callee(i32 %a, i1 %b) {
-  call void asm sideeffect "s_nop 1\0A\09.pushsection other\0A\09s_nop 2\0A\09s_nop 3\0A\09.popsection\0A\09s_nop 4\0A\09.align 32", ""()
+  call void asm sideeffect "s_nop 1 # some comment ; still comment \09.pushsection other; s_nop 2 \0A s_nop 3 \0A.popsection\0A s_nop 4; label:\0A", ""()
   ret void
 }
 ; CHECK: define void @callee