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

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;
+
+  /// Parses the assembly instruction and returns individual non-empty
+  /// instructions in a vector. Treats '\n' as instruction separator, but not
+  /// ';' (due to conflict with MachO comment).
+  /// 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

@@ -10006,8 +10006,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) {
@@ -10051,7 +10050,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,23 @@ FunctionType *InlineAsm::getFunctionType() const {
   return FTy;
 }
 
-void InlineAsm::collectAsmStrs(SmallVectorImpl<StringRef> &AsmStrs) const {
+SmallVector<StringRef> InlineAsm::collectAsmInstrs() const {
   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);
+  SmallVector<StringRef> AsmLines;
+  AsmStr.split(AsmLines, '\n');
+
+  SmallVector<StringRef> AsmInstrs;
+  AsmInstrs.reserve(AsmLines.size());
+  for (StringRef AsmLine : AsmLines) {
+    // Trim most general comments. We don't handle comment blocks (/* ... */).
+    // We also don't handle '@' (ARM) and ';' (MachO) since they have different
+    // interpretations in different targets and we don't have target info in IR.
+    auto Trimmed = AsmLine.split('#').first.split("//").first.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/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 \0A.pushsection other\0A s_nop 2 \0A s_nop 3 \0A.popsection\0A s_nop 4\0A label:\0A", ""()
   ret void
 }
 ; CHECK: define void @callee