[PATCH] D89892: [AsmPrinter] Add per BB instruction mix remark.

Jessica Paquette via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 21 15:30:40 PDT 2020


paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1208
         emitInstruction(&MI);
+        if (CanDoExtraAnalysis) {
+          auto I = OpcodeCounts.insert({MI.getOpcode(), 0u});
----------------
Will this work with inline assembly? Looks like inline assembly is handled earlier in the switch.

(Probably fine to support it later anyway.)


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1258
+      // Skip empty blocks.
+      if (MBB.begin() == MBB.end())
+        continue;
----------------
`MBB.empty()`?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1268
+      for (auto &KV : OpcodeCounts) {
+        auto Name = (Twine("INST_") + TII->getName(KV.first)).str();
+        OpcodeCountsVec.emplace_back(Name, KV.second);
----------------
Is there any reason that you're prefixing these with `INST_`? Are you planning on adding other ways to categorise these?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89892



More information about the llvm-commits mailing list