From: Andrea Di Biagio Date: Thu, 9 May 2019 15:18:09 +0000 (+0000) Subject: [MCA] Add support for nested and overlapping region markers X-Git-Tag: android-x86-9.0-r1~3635 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=9d3a058db3e6127fd9019898c51c299d9837b312;p=android-x86%2Fexternal-llvm.git [MCA] Add support for nested and overlapping region markers 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 --- diff --git a/docs/CommandGuide/llvm-mca.rst b/docs/CommandGuide/llvm-mca.rst index 5d504a9f9cb..5b30a8a532a 100644 --- a/docs/CommandGuide/llvm-mca.rst +++ b/docs/CommandGuide/llvm-mca.rst @@ -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 index 00000000000..8420f03bf0c --- /dev/null +++ b/test/tools/llvm-mca/X86/llvm-mca-markers-10.s @@ -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 index 00000000000..6f957818433 --- /dev/null +++ b/test/tools/llvm-mca/X86/llvm-mca-markers-11.s @@ -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 index 00000000000..cf45d6bf6c9 --- /dev/null +++ b/test/tools/llvm-mca/X86/llvm-mca-markers-12.s @@ -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: ^ diff --git a/test/tools/llvm-mca/X86/llvm-mca-markers-6.s b/test/tools/llvm-mca/X86/llvm-mca-markers-6.s index 27441a5a2e9..22f18f0e06a 100644 --- a/test/tools/llvm-mca/X86/llvm-mca-markers-6.s +++ b/test/tools/llvm-mca/X86/llvm-mca-markers-6.s @@ -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. diff --git a/test/tools/llvm-mca/X86/llvm-mca-markers-7.s b/test/tools/llvm-mca/X86/llvm-mca-markers-7.s index c6fc5c06a30..4d07e745628 100644 --- a/test/tools/llvm-mca/X86/llvm-mca-markers-7.s +++ b/test/tools/llvm-mca/X86/llvm-mca-markers-7.s @@ -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 index 00000000000..5bc57168200 --- /dev/null +++ b/test/tools/llvm-mca/X86/llvm-mca-markers-8.s @@ -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 index 00000000000..37b51e3513f --- /dev/null +++ b/test/tools/llvm-mca/X86/llvm-mca-markers-9.s @@ -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 diff --git a/tools/llvm-mca/CodeRegion.cpp b/tools/llvm-mca/CodeRegion.cpp index eb04ea3f0d8..bf592f67245 100644 --- a/tools/llvm-mca/CodeRegion.cpp +++ b/tools/llvm-mca/CodeRegion.cpp @@ -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("", 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(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(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 diff --git a/tools/llvm-mca/CodeRegion.h b/tools/llvm-mca/CodeRegion.h index 80532e311ed..cabb4a5d448 100644 --- a/tools/llvm-mca/CodeRegion.h +++ b/tools/llvm-mca/CodeRegion.h @@ -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; std::vector Regions; + llvm::StringMap 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 diff --git a/tools/llvm-mca/CodeRegionGenerator.cpp b/tools/llvm-mca/CodeRegionGenerator.cpp index 14b470a417c..c793169e64e 100644 --- a/tools/llvm-mca/CodeRegionGenerator.cpp +++ b/tools/llvm-mca/CodeRegionGenerator.cpp @@ -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; } diff --git a/tools/llvm-mca/llvm-mca.cpp b/tools/llvm-mca/llvm-mca.cpp index 861b6a5809a..e70c8f627ef 100644 --- a/tools/llvm-mca/llvm-mca.cpp +++ b/tools/llvm-mca/llvm-mca.cpp @@ -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;