[PATCH] D109661: [FunctionPropertiesAnalysis] Add `PreciseFunctionPropertiesAnalysis`

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/include/llvm/Analysis/FunctionPropertiesAnalysis.h:92
+
+  void visitInstruction(Instruction &I) {
+    errs() << "Instruction Count does not know about " << I;
----------------
does the visitor need to be public?


================
Comment at: llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp:143
+
+    if (BB.size() > 500)
+      FPI.incrementBigBasicBlock();
----------------
make 500 a flag (it's arbitrary otherwise. well... it's arbitrary after that, too, but at least it's user-controllable.). same with 15 below


================
Comment at: llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp:156
+
+      for (unsigned int i = 0; i < I.getNumOperands(); i++)
+        if (auto *C = dyn_cast<Constant>(I.getOperand(i))) {
----------------
style: capitalize variables. since you have an "I", maybe s/i/OpIdx


================
Comment at: llvm/lib/Analysis/FunctionPropertiesAnalysis.cpp:171
+  for (const auto &BB : F) {
+    int LoopDepth = LI.getLoopDepth(&BB);
+    if (FPI.getMaxLoopDepth() < LoopDepth)
----------------
unsigned LoopDepth


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

https://reviews.llvm.org/D109661