This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by zjaffal on Apr 14 2023, 1:34 PM.

Diff Detail

Event Timeline

zjaffal created this revision.Apr 14 2023, 1:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2023, 1:34 PM
Herald added a subscriber: StephenFan. · View Herald Transcript
zjaffal requested review of this revision.Apr 14 2023, 1:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2023, 1:34 PM

Why not use the YAML output directly in this case?

Why not use the YAML output directly in this case?

If you want to generate the count for functions there is the possibility that the function name is not unique so you will end up with different counts for the same function. While the DebugLoc is not unique but it is better than just using the function name as an identifier.

paquette added inline comments.Apr 14 2023, 2:20 PM
llvm/test/tools/llvm-remarkutil/annotation-count-with-dbg-loc.test
8

add newline

llvm/tools/llvm-remarkutil/RemarkUtil.cpp
265

We can reduce duplication in the output string here:

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

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 << ...;
316

Same here

325

I think we can factor this out into a "shouldSkipRemark" and share it between the instruction count + annotation remark modes.

342

Same as other feedback for string output

zjaffal updated this revision to Diff 514139.Apr 17 2023, 1:53 AM

refactor code to reduce duplication

This revision is now accepted and ready to land.Apr 17 2023, 1:09 PM
This revision was landed with ongoing or failed builds.Apr 18 2023, 5:49 AM
This revision was automatically updated to reflect the committed changes.