[PATCH] D148374: [llvm-remarkutil] Add an option to display DebugLoc when collecting counts for remarks.

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/test/tools/llvm-remarkutil/annotation-count-with-dbg-loc.test:8
+; CHECK: path/to/anno3.c:1:2,func3,3
\ No newline at end of file

----------------
add newline


================
Comment at: llvm/tools/llvm-remarkutil/RemarkUtil.cpp:259
   // Emit CSV header.
-  OF->os() << "Function,InstructionCount\n";
+  if (UseDebugLoc)
+    OF->os() << "Source,Function,InstructionCount\n";
----------------
We can reduce duplication in the output string here:

```
if (UseDebugLoc)
  OF->os() << "Source,"
OF->os() << "Function,InstructionCount\n";
```


================
Comment at: llvm/tools/llvm-remarkutil/RemarkUtil.cpp:278
            "Expected instruction count remarks to have a NumInstructions key?");
-    OF->os() << Remark.FunctionName << "," << InstrCountArg->Val << "\n";
+    if (UseDebugLoc) {
+      std::string Loc = Remark.Loc->SourceFilePath.str() + ":" +
----------------
Similar to the case above, we can just emit the debug location here, and then eliminate the `else`.

```
// Emit debug location if the user wants it.
if (UseDebugLoc) {
  std::string Loc = ...;
  OF->os() << Loc << ",";
}
// Always emit function name and instruction count.
OF->os() << Remark.FunctionName << ...;
```


================
Comment at: llvm/tools/llvm-remarkutil/RemarkUtil.cpp:312
   // Emit CSV header.
-  OF->os() << "Function,Count\n";
+  if (UseDebugLoc)
+    OF->os() << "Source,Function,Count\n";
----------------
Same here


================
Comment at: llvm/tools/llvm-remarkutil/RemarkUtil.cpp:324
       continue;
+    if (UseDebugLoc && !Remark.Loc.has_value())
+      continue;
----------------
I think we can factor this out into a "shouldSkipRemark" and share it between the instruction count + annotation remark modes.


================
Comment at: llvm/tools/llvm-remarkutil/RemarkUtil.cpp:335
            "Expected annotation-type remark to have a count key?");
-    OF->os() << Remark.FunctionName << "," << CountArg->Val << "\n";
+    if (UseDebugLoc) {
+      std::string Loc = Remark.Loc->SourceFilePath.str() + ":" +
----------------
Same as other feedback for string output


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148374/new/

https://reviews.llvm.org/D148374