[PATCH] D53585: [llvm-mca] Improved error handling and error reporting from class InstrBuilder.

[Matt Davis via Phabricator via llvm-commits]

mattd added a comment.

LGTM.  I have a few comments in line, but the code looks nice.



================
Comment at: tools/llvm-mca/lib/InstrBuilder.cpp:334
 
-  std::string ToString;
-  raw_string_ostream OS(ToString);
+  std::string Message;
   if (UsesMemory) {
----------------
You can probably get away with just making these StringRefs in this case.


================
Comment at: tools/llvm-mca/lib/InstrBuilder.cpp:341
+    Message = "found an inconsistent instruction that decodes"
+              " to zero opcodes and that consumes scheduler "
+              "resources.";
----------------
I know the previous message contents are the same, but for consistency, could you add the space on the preceeding line, instead of continuing the message with a leading space.

```"found an inconsistent instruction that decodes "
"to zero opcodes and that consumes scheduler "
```


================
Comment at: tools/llvm-mca/llvm-mca.cpp:331
+// Returns true on success.
+bool runPipeline(mca::Pipeline &P, MCInstPrinter &MCIP, MCSubtargetInfo &STI) {
+  // Handle pipeline errors here.
----------------
This function can be marked as a static.
STI can be made const.


https://reviews.llvm.org/D53585