[PATCH] D33608: [Analysis] Rewrite TotalMemInst counting in InstCount pass in a way that doesn't require reading back other Statistic variables

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Analysis/InstCount.cpp:44-56
 #define HANDLE_INST(N, OPCODE, CLASS) \
-    void visit##OPCODE(CLASS &) { ++Num##OPCODE##Inst; ++TotalInsts; }
+    void visit##OPCODE(CLASS &) { \
+      ++Num##OPCODE##Inst; ++TotalInsts; \
+      if (N == Instruction::GetElementPtr || \
+          N == Instruction::Load || \
+          N == Instruction::Store || \
+          N == Instruction::Call || \
----------------
This seems... a little crazy.

First off, we don't need to increment TotalInsts here. You can just override `visitInstruction` and teach this to delegate to the base class after incrementing the opcode specific counter. But maybe that doesn't really matter...

But the whole point of InstVisitor is to avoid these long || chains. =[

And what even is a "memory instruction"? Why is a call a memory instruction even if it is calling an LLVM intrinsic? I know you didn't add any of this, but it seems somewhat crazy to bend over backwards to keep it.

How about just deleting the `TotalMemInst` statistic entirely until someone comes back with a specific use case?


https://reviews.llvm.org/D33608