OSDN Git Service

[MCA] Add support for nested and overlapping region markers
authorAndrea Di Biagio <Andrea_DiBiagio@sn.scee.net>
Thu, 9 May 2019 15:18:09 +0000 (15:18 +0000)
committerAndrea Di Biagio <Andrea_DiBiagio@sn.scee.net>
Thu, 9 May 2019 15:18:09 +0000 (15:18 +0000)
This patch fixes PR41523
https://bugs.llvm.org/show_bug.cgi?id=41523

Regions can now nest/overlap provided that they have different names.
Anonymous regions cannot overlap.

Region end markers must specify the region name. The only exception is for when
there is only one user-defined region; in that particular case, the region end
marker doesn't need to specify a name.

Incorrect region end markers are no longer ignored. Instead, the tool reports an
error and we exit with an error code.

Added test cases to verify the new diagnostic error messages.

Updated the llvm-mca docs to reflect this feature change.

Differential Revision: https://reviews.llvm.org/D61676

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@360351 91177308-0d34-0410-b5e6-96231b3b80d8

12 files changed:
docs/CommandGuide/llvm-mca.rst
test/tools/llvm-mca/X86/llvm-mca-markers-10.s [new file with mode: 0644]
test/tools/llvm-mca/X86/llvm-mca-markers-11.s [new file with mode: 0644]
test/tools/llvm-mca/X86/llvm-mca-markers-12.s [new file with mode: 0644]
test/tools/llvm-mca/X86/llvm-mca-markers-6.s
test/tools/llvm-mca/X86/llvm-mca-markers-7.s
test/tools/llvm-mca/X86/llvm-mca-markers-8.s [new file with mode: 0644]
test/tools/llvm-mca/X86/llvm-mca-markers-9.s [new file with mode: 0644]
tools/llvm-mca/CodeRegion.cpp
tools/llvm-mca/CodeRegion.h
tools/llvm-mca/CodeRegionGenerator.cpp
tools/llvm-mca/llvm-mca.cpp

index 5d504a9..5b30a8a 100644 (file)
@@ -192,16 +192,54 @@ example:
 
 .. code-block:: none
 
-  # LLVM-MCA-BEGIN My Code Region
+  # LLVM-MCA-BEGIN
     ...
   # LLVM-MCA-END
 
-Multiple regions can be specified provided that they do not overlap.  A code
-region can have an optional description. If no user-defined region is specified,
-then :program:`llvm-mca` assumes a default region which contains every
-instruction in the input file.  Every region is analyzed in isolation, and the
-final performance report is the union of all the reports generated for every
-code region.
+If no user-defined region is specified, then :program:`llvm-mca` assumes a
+default region which contains every instruction in the input file.  Every region
+is analyzed in isolation, and the final performance report is the union of all
+the reports generated for every code region.
+
+Code regions can have names. For example:
+
+.. code-block:: none
+
+  # LLVM-MCA-BEGIN A simple example
+    add %eax, %eax
+  # LLVM-MCA-END 
+
+The code from the example above defines a region named "A simple example" with a
+single instruction in it. Note how the region name doesn't have to be repeated
+in the ``LLVM-MCA-END`` directive. In the absence of overlapping regions,
+an anonymous ``LLVM-MCA-END`` directive always ends the currently active user
+defined region.
+
+Example of nesting regions:
+
+.. code-block:: none
+
+  # LLVM-MCA-BEGIN foo
+    add %eax, %edx
+  # LLVM-MCA-BEGIN bar
+    sub %eax, %edx
+  # LLVM-MCA-END bar
+  # LLVM-MCA-END foo
+
+Example of overlapping regions:
+
+.. code-block:: none
+
+  # LLVM-MCA-BEGIN foo
+    add %eax, %edx
+  # LLVM-MCA-BEGIN bar
+    sub %eax, %edx
+  # LLVM-MCA-END foo
+    add %eax, %edx
+  # LLVM-MCA-END bar
+
+Note that multiple anonymous regions cannot overlap. Also, overlapping regions
+cannot have the same name.
 
 Inline assembly directives may be used from source code to annotate the
 assembly text:
diff --git a/test/tools/llvm-mca/X86/llvm-mca-markers-10.s b/test/tools/llvm-mca/X86/llvm-mca-markers-10.s
new file mode 100644 (file)
index 0000000..8420f03
--- /dev/null
@@ -0,0 +1,110 @@
+# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
+# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 < %s | FileCheck %s
+
+testloop:
+# LLVM-MCA-BEGIN upper
+  leal 42(%rdi), %eax
+# LLVM-MCA-BEGIN lower
+  imull %esi, %eax
+# LLVM-MCA-END upper
+  leal 42(%rdi), %eax
+# LLVM-MCA-END lower
+  imull %esi, %eax
+
+# CHECK:      [0] Code Region - upper
+
+# CHECK:      Iterations:        100
+# CHECK-NEXT: Instructions:      200
+# CHECK-NEXT: Total Cycles:      205
+# CHECK-NEXT: Total uOps:        300
+
+# CHECK:      Dispatch Width:    2
+# CHECK-NEXT: uOps Per Cycle:    1.46
+# CHECK-NEXT: IPC:               0.98
+# CHECK-NEXT: Block RThroughput: 1.5
+
+# 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.50                        leal   42(%rdi), %eax
+# CHECK-NEXT:  2      3     1.00                        imull  %esi, %eax
+
+# CHECK:      Resources:
+# CHECK-NEXT: [0]   - JALU0
+# CHECK-NEXT: [1]   - JALU1
+# CHECK-NEXT: [2]   - JDiv
+# CHECK-NEXT: [3]   - JFPA
+# CHECK-NEXT: [4]   - JFPM
+# CHECK-NEXT: [5]   - JFPU0
+# CHECK-NEXT: [6]   - JFPU1
+# CHECK-NEXT: [7]   - JLAGU
+# CHECK-NEXT: [8]   - JMul
+# CHECK-NEXT: [9]   - JSAGU
+# CHECK-NEXT: [10]  - JSTC
+# CHECK-NEXT: [11]  - JVALU0
+# CHECK-NEXT: [12]  - JVALU1
+# CHECK-NEXT: [13]  - JVIMUL
+
+# CHECK:      Resource pressure per iteration:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]    [8]    [9]    [10]   [11]   [12]   [13]
+# CHECK-NEXT: 0.99   1.01    -      -      -      -      -      -     1.00    -      -      -      -      -
+
+# CHECK:      Resource pressure by instruction:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]    [8]    [9]    [10]   [11]   [12]   [13]   Instructions:
+# CHECK-NEXT: 0.99   0.01    -      -      -      -      -      -      -      -      -      -      -      -     leal   42(%rdi), %eax
+# CHECK-NEXT:  -     1.00    -      -      -      -      -      -     1.00    -      -      -      -      -     imull  %esi, %eax
+
+# CHECK:      [1] Code Region - lower
+
+# CHECK:      Iterations:        100
+# CHECK-NEXT: Instructions:      200
+# CHECK-NEXT: Total Cycles:      204
+# CHECK-NEXT: Total uOps:        300
+
+# CHECK:      Dispatch Width:    2
+# CHECK-NEXT: uOps Per Cycle:    1.47
+# CHECK-NEXT: IPC:               0.98
+# CHECK-NEXT: Block RThroughput: 1.5
+
+# 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:  2      3     1.00                        imull  %esi, %eax
+# CHECK-NEXT:  1      1     0.50                        leal   42(%rdi), %eax
+
+# CHECK:      Resources:
+# CHECK-NEXT: [0]   - JALU0
+# CHECK-NEXT: [1]   - JALU1
+# CHECK-NEXT: [2]   - JDiv
+# CHECK-NEXT: [3]   - JFPA
+# CHECK-NEXT: [4]   - JFPM
+# CHECK-NEXT: [5]   - JFPU0
+# CHECK-NEXT: [6]   - JFPU1
+# CHECK-NEXT: [7]   - JLAGU
+# CHECK-NEXT: [8]   - JMul
+# CHECK-NEXT: [9]   - JSAGU
+# CHECK-NEXT: [10]  - JSTC
+# CHECK-NEXT: [11]  - JVALU0
+# CHECK-NEXT: [12]  - JVALU1
+# CHECK-NEXT: [13]  - JVIMUL
+
+# CHECK:      Resource pressure per iteration:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]    [8]    [9]    [10]   [11]   [12]   [13]
+# CHECK-NEXT: 1.00   1.00    -      -      -      -      -      -     1.00    -      -      -      -      -
+
+# CHECK:      Resource pressure by instruction:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]    [8]    [9]    [10]   [11]   [12]   [13]   Instructions:
+# CHECK-NEXT:  -     1.00    -      -      -      -      -      -     1.00    -      -      -      -      -     imull  %esi, %eax
+# CHECK-NEXT: 1.00    -      -      -      -      -      -      -      -      -      -      -      -      -     leal   42(%rdi), %eax
diff --git a/test/tools/llvm-mca/X86/llvm-mca-markers-11.s b/test/tools/llvm-mca/X86/llvm-mca-markers-11.s
new file mode 100644 (file)
index 0000000..6f95781
--- /dev/null
@@ -0,0 +1,13 @@
+# RUN: not llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 %s 2>&1 | FileCheck %s
+
+# LLVM-MCA-BEGIN foo
+add %eax, %eax
+# LLVM-MCA-BEGIN foo
+add %eax, %eax
+
+# CHECK:      llvm-mca-markers-11.s:5:2: error: overlapping regions cannot have the same name
+# CHECK-NEXT: # LLVM-MCA-BEGIN foo
+# CHECK-NEXT:  ^
+# CHECK-NEXT: llvm-mca-markers-11.s:3:2: note: region foo was previously defined here
+# CHECK-NEXT: # LLVM-MCA-BEGIN foo
+# CHECK-NEXT:  ^
diff --git a/test/tools/llvm-mca/X86/llvm-mca-markers-12.s b/test/tools/llvm-mca/X86/llvm-mca-markers-12.s
new file mode 100644 (file)
index 0000000..cf45d6b
--- /dev/null
@@ -0,0 +1,13 @@
+# RUN: not llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 %s 2>&1 | FileCheck %s
+
+# LLVM-MCA-BEGIN
+add %eax, %eax
+# LLVM-MCA-BEGIN
+add %eax, %eax
+
+# CHECK:      llvm-mca-markers-12.s:5:2: error: found multiple overlapping anonymous regions
+# CHECK-NEXT: # LLVM-MCA-BEGIN
+# CHECK-NEXT:  ^
+# CHECK-NEXT: llvm-mca-markers-12.s:3:2: note: Previous anonymous region was defined here
+# CHECK-NEXT: # LLVM-MCA-BEGIN
+# CHECK-NEXT:  ^
index 27441a5..22f18f0 100644 (file)
@@ -6,7 +6,9 @@
 
 # LLVM-MCA-END
 
-# CHECK:      llvm-mca-markers-6.s:5:2: warning: Ignoring invalid region start
-# CHECK-NEXT: # LLVM-MCA-BEGIN  bar
+# CHECK:      llvm-mca-markers-6.s:7:2: error: found an invalid region end directive
+# CHECK-NEXT: # LLVM-MCA-END
+# CHECK-NEXT:  ^
+# CHECK-NEXT: llvm-mca-markers-6.s:7:2: note: unable to find an active anonymous region
+# CHECK-NEXT: # LLVM-MCA-END
 # CHECK-NEXT:  ^
-# CHECK-NEXT: error: no assembly instructions found.
index c6fc5c0..4d07e74 100644 (file)
@@ -6,6 +6,9 @@
 
 # LLVM-MCA-END
 
-# CHECK:      llvm-mca-markers-7.s:7:2: warning: Ignoring invalid region end
+# CHECK:      llvm-mca-markers-7.s:7:2: error: found an invalid region end directive
+# CHECK-NEXT: # LLVM-MCA-END
+# CHECK-NEXT:  ^
+# CHECK-NEXT: llvm-mca-markers-7.s:7:2: note: unable to find an active anonymous region
 # CHECK-NEXT: # LLVM-MCA-END
 # CHECK-NEXT:  ^
diff --git a/test/tools/llvm-mca/X86/llvm-mca-markers-8.s b/test/tools/llvm-mca/X86/llvm-mca-markers-8.s
new file mode 100644 (file)
index 0000000..5bc5716
--- /dev/null
@@ -0,0 +1,10 @@
+# RUN: not llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 %s 2>&1 | FileCheck %s
+
+# LLVM-MCA-END foo
+
+# CHECK:      llvm-mca-markers-8.s:3:2: error: found an invalid region end directive
+# CHECK-NEXT: # LLVM-MCA-END foo
+# CHECK-NEXT:  ^
+# CHECK-NEXT: llvm-mca-markers-8.s:3:2: note: unable to find an active region named foo
+# CHECK-NEXT: # LLVM-MCA-END foo
+# CHECK-NEXT:  ^
diff --git a/test/tools/llvm-mca/X86/llvm-mca-markers-9.s b/test/tools/llvm-mca/X86/llvm-mca-markers-9.s
new file mode 100644 (file)
index 0000000..37b51e3
--- /dev/null
@@ -0,0 +1,110 @@
+# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
+# RUN: llvm-mca -mtriple=x86_64-unknown-unknown -mcpu=btver2 < %s | FileCheck %s
+
+testloop:
+# LLVM-MCA-BEGIN outer
+  leal 42(%rdi), %eax
+# LLVM-MCA-BEGIN inner
+  imull %esi, %eax
+# LLVM-MCA-END inner
+  leal 42(%rdi), %eax
+# LLVM-MCA-END outer
+  imull %esi, %eax
+
+# CHECK:      [0] Code Region - outer
+
+# CHECK:      Iterations:        100
+# CHECK-NEXT: Instructions:      300
+# CHECK-NEXT: Total Cycles:      205
+# CHECK-NEXT: Total uOps:        400
+
+# CHECK:      Dispatch Width:    2
+# CHECK-NEXT: uOps Per Cycle:    1.95
+# CHECK-NEXT: IPC:               1.46
+# CHECK-NEXT: Block RThroughput: 2.0
+
+# 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.50                        leal   42(%rdi), %eax
+# CHECK-NEXT:  2      3     1.00                        imull  %esi, %eax
+# CHECK-NEXT:  1      1     0.50                        leal   42(%rdi), %eax
+
+# CHECK:      Resources:
+# CHECK-NEXT: [0]   - JALU0
+# CHECK-NEXT: [1]   - JALU1
+# CHECK-NEXT: [2]   - JDiv
+# CHECK-NEXT: [3]   - JFPA
+# CHECK-NEXT: [4]   - JFPM
+# CHECK-NEXT: [5]   - JFPU0
+# CHECK-NEXT: [6]   - JFPU1
+# CHECK-NEXT: [7]   - JLAGU
+# CHECK-NEXT: [8]   - JMul
+# CHECK-NEXT: [9]   - JSAGU
+# CHECK-NEXT: [10]  - JSTC
+# CHECK-NEXT: [11]  - JVALU0
+# CHECK-NEXT: [12]  - JVALU1
+# CHECK-NEXT: [13]  - JVIMUL
+
+# CHECK:      Resource pressure per iteration:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]    [8]    [9]    [10]   [11]   [12]   [13]
+# CHECK-NEXT: 1.00   2.00    -      -      -      -      -      -     1.00    -      -      -      -      -
+
+# CHECK:      Resource pressure by instruction:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]    [8]    [9]    [10]   [11]   [12]   [13]   Instructions:
+# CHECK-NEXT:  -     1.00    -      -      -      -      -      -      -      -      -      -      -      -     leal   42(%rdi), %eax
+# CHECK-NEXT:  -     1.00    -      -      -      -      -      -     1.00    -      -      -      -      -     imull  %esi, %eax
+# CHECK-NEXT: 1.00    -      -      -      -      -      -      -      -      -      -      -      -      -     leal   42(%rdi), %eax
+
+# CHECK:      [1] Code Region - inner
+
+# CHECK:      Iterations:        100
+# CHECK-NEXT: Instructions:      100
+# CHECK-NEXT: Total Cycles:      303
+# CHECK-NEXT: Total uOps:        200
+
+# CHECK:      Dispatch Width:    2
+# CHECK-NEXT: uOps Per Cycle:    0.66
+# CHECK-NEXT: IPC:               0.33
+# CHECK-NEXT: Block RThroughput: 1.0
+
+# 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:  2      3     1.00                        imull  %esi, %eax
+
+# CHECK:      Resources:
+# CHECK-NEXT: [0]   - JALU0
+# CHECK-NEXT: [1]   - JALU1
+# CHECK-NEXT: [2]   - JDiv
+# CHECK-NEXT: [3]   - JFPA
+# CHECK-NEXT: [4]   - JFPM
+# CHECK-NEXT: [5]   - JFPU0
+# CHECK-NEXT: [6]   - JFPU1
+# CHECK-NEXT: [7]   - JLAGU
+# CHECK-NEXT: [8]   - JMul
+# CHECK-NEXT: [9]   - JSAGU
+# CHECK-NEXT: [10]  - JSTC
+# CHECK-NEXT: [11]  - JVALU0
+# CHECK-NEXT: [12]  - JVALU1
+# CHECK-NEXT: [13]  - JVIMUL
+
+# CHECK:      Resource pressure per iteration:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]    [8]    [9]    [10]   [11]   [12]   [13]
+# CHECK-NEXT:  -     1.00    -      -      -      -      -      -     1.00    -      -      -      -      -
+
+# CHECK:      Resource pressure by instruction:
+# CHECK-NEXT: [0]    [1]    [2]    [3]    [4]    [5]    [6]    [7]    [8]    [9]    [10]   [11]   [12]   [13]   Instructions:
+# CHECK-NEXT:  -     1.00    -      -      -      -      -      -     1.00    -      -      -      -      -     imull  %esi, %eax
index eb04ea3..bf592f6 100644 (file)
@@ -16,7 +16,7 @@
 namespace llvm {
 namespace mca {
 
-CodeRegions::CodeRegions(llvm::SourceMgr &S) : SM(S) {
+CodeRegions::CodeRegions(llvm::SourceMgr &S) : SM(S), FoundErrors(false) {
   // Create a default region for the input code sequence.
   Regions.emplace_back(make_unique<CodeRegion>("", SMLoc()));
 }
@@ -30,41 +30,87 @@ bool CodeRegion::isLocInRange(SMLoc Loc) const {
 }
 
 void CodeRegions::beginRegion(StringRef Description, SMLoc Loc) {
-  assert(!Regions.empty() && "Missing Default region");
-  const CodeRegion &CurrentRegion = *Regions.back();
-  if (CurrentRegion.startLoc().isValid() && !CurrentRegion.endLoc().isValid()) {
-    SM.PrintMessage(Loc, SourceMgr::DK_Warning,
-                    "Ignoring invalid region start");
-    return;
+  if (ActiveRegions.empty()) {
+    // Remove the default region if there is at least one user defined region.
+    // By construction, only the default region has an invalid start location.
+    if (Regions.size() == 1 && !Regions[0]->startLoc().isValid() &&
+        !Regions[0]->endLoc().isValid()) {
+      ActiveRegions[Description] = 0;
+      Regions[0] = make_unique<CodeRegion>(Description, Loc);
+      return;
+    }
+  } else {
+    auto It = ActiveRegions.find(Description);
+    if (It != ActiveRegions.end()) {
+      const CodeRegion &R = *Regions[It->second];
+      if (Description.empty()) {
+        SM.PrintMessage(Loc, SourceMgr::DK_Error,
+                        "found multiple overlapping anonymous regions");
+        SM.PrintMessage(R.startLoc(), SourceMgr::DK_Note,
+                        "Previous anonymous region was defined here");
+        FoundErrors = true;
+        return;
+      }
+
+      SM.PrintMessage(Loc, SourceMgr::DK_Error,
+                      "overlapping regions cannot have the same name");
+      SM.PrintMessage(R.startLoc(), SourceMgr::DK_Note,
+                      "region " + Description + " was previously defined here");
+      FoundErrors = true;
+      return;
+    }
   }
 
-  // Remove the default region if there are user defined regions.
-  if (!CurrentRegion.startLoc().isValid())
-    Regions.erase(Regions.begin());
+  ActiveRegions[Description] = Regions.size();
   Regions.emplace_back(make_unique<CodeRegion>(Description, Loc));
+  return;
 }
 
-void CodeRegions::endRegion(SMLoc Loc) {
-  assert(!Regions.empty() && "Missing Default region");
-  CodeRegion &CurrentRegion = *Regions.back();
-  if (CurrentRegion.endLoc().isValid()) {
-    SM.PrintMessage(Loc, SourceMgr::DK_Warning,
-                    "Ignoring invalid region end");
+void CodeRegions::endRegion(StringRef Description, SMLoc Loc) {
+  if (Description.empty()) {
+    // Special case where there is only one user defined region,
+    // and this LLVM-MCA-END directive doesn't provide a region name.
+    // In this case, we assume that the user simply wanted to just terminate
+    // the only active region.
+    if (ActiveRegions.size() == 1) {
+      auto It = ActiveRegions.begin();
+      Regions[It->second]->setEndLocation(Loc);
+      ActiveRegions.erase(It);
+      return;
+    }
+
+    // Special case where the region end marker applies to the default region.
+    if (ActiveRegions.empty() && Regions.size() == 1 &&
+        !Regions[0]->startLoc().isValid() && !Regions[0]->endLoc().isValid()) {
+      Regions[0]->setEndLocation(Loc);
+      return;
+    }
+  }
+
+  auto It = ActiveRegions.find(Description);
+  if (It != ActiveRegions.end()) {
+    Regions[It->second]->setEndLocation(Loc);
+    ActiveRegions.erase(It);
     return;
   }
 
-  CurrentRegion.setEndLocation(Loc);
+  FoundErrors = true;
+  SM.PrintMessage(Loc, SourceMgr::DK_Error,
+                  "found an invalid region end directive");
+  if (!Description.empty()) {
+    SM.PrintMessage(Loc, SourceMgr::DK_Note,
+                    "unable to find an active region named " + Description);
+  } else {
+    SM.PrintMessage(Loc, SourceMgr::DK_Note,
+                    "unable to find an active anonymous region");
+  }
 }
 
 void CodeRegions::addInstruction(const MCInst &Instruction) {
-  const SMLoc &Loc = Instruction.getLoc();
-  const auto It =
-      std::find_if(Regions.rbegin(), Regions.rend(),
-                   [Loc](const UniqueCodeRegion &Region) {
-                     return Region->isLocInRange(Loc);
-                   });
-  if (It != Regions.rend())
-    (*It)->addInstruction(Instruction);
+  SMLoc Loc = Instruction.getLoc();
+  for (UniqueCodeRegion &Region : Regions)
+    if (Region->isLocInRange(Loc))
+      Region->addInstruction(Instruction);
 }
 
 } // namespace mca
index 80532e3..cabb4a5 100644 (file)
@@ -79,12 +79,16 @@ public:
   llvm::StringRef getDescription() const { return Description; }
 };
 
+class CodeRegionParseError final : public Error {};
+
 class CodeRegions {
   // A source manager. Used by the tool to generate meaningful warnings.
   llvm::SourceMgr &SM;
 
   using UniqueCodeRegion = std::unique_ptr<CodeRegion>;
   std::vector<UniqueCodeRegion> Regions;
+  llvm::StringMap<unsigned> ActiveRegions;
+  bool FoundErrors;
 
   CodeRegions(const CodeRegions &) = delete;
   CodeRegions &operator=(const CodeRegions &) = delete;
@@ -101,7 +105,7 @@ public:
   const_iterator end() const { return Regions.cend(); }
 
   void beginRegion(llvm::StringRef Description, llvm::SMLoc Loc);
-  void endRegion(llvm::SMLoc Loc);
+  void endRegion(llvm::StringRef Description, llvm::SMLoc Loc);
   void addInstruction(const llvm::MCInst &Instruction);
   llvm::SourceMgr &getSourceMgr() const { return SM; }
 
@@ -114,6 +118,8 @@ public:
       return Region->empty();
     });
   }
+
+  bool isValid() const { return !FoundErrors; }
 };
 
 } // namespace mca
index 14b470a..c793169 100644 (file)
@@ -86,7 +86,11 @@ void MCACommentConsumer::HandleComment(SMLoc Loc, StringRef CommentText) {
 
   Comment = Comment.drop_front(Position);
   if (Comment.consume_front("LLVM-MCA-END")) {
-    Regions.endRegion(Loc);
+    // Skip spaces and tabs.
+    Position = Comment.find_first_not_of(" \t");
+    if (Position < Comment.size())
+      Comment = Comment.drop_front(Position);
+    Regions.endRegion(Comment, Loc);
     return;
   }
 
index 861b6a5..e70c8f6 100644 (file)
@@ -364,6 +364,11 @@ int main(int argc, char **argv) {
     return 1;
   }
   const mca::CodeRegions &Regions = *RegionsOrErr;
+
+  // Early exit if errors were found by the code region parsing logic.
+  if (!Regions.isValid())
+    return 1;
+
   if (Regions.empty()) {
     WithColor::error() << "no assembly instructions found.\n";
     return 1;