[PATCH] D38768: Add remarks describing when a pass changes the IR instruction count of a module

[Adam Nemet via Phabricator via llvm-commits]

anemet added a comment.

Mostly small things except for the question on whether we should only compute this when the remark is actually enabled.



================
Comment at: include/llvm/IR/Function.h:144-145
 
+  /// Returns the number of IR instructions contained in the \p Function.
+  int getIRCount();
+
----------------
getInstructionCount is probably a better name?  Later to you have more IR->Instruction.

int -> unsigned?

Remove \p since this it's not a parameter?


================
Comment at: lib/Analysis/CallGraphSCCPass.cpp:135
+      // Remember the number of instructions in the module before the pass.
+      int OriginalCount = M.getIRCount();
       Changed = CGSP->runOnSCC(CurSCC);
----------------
How expensive is this in practice (I know that it's linear but we have many passes)?  You may want to do this only if the remark is requested.

You should be able to check this with the DiagnosticHandler in the LLVMContext.


================
Comment at: lib/IR/LegacyPassManager.cpp:139-140
+void PMDataManager::emitIRSizeChangedRemark(Pass *P, Module &M,
+                                            const int &OriginalCount,
+                                            const int &FinalCount) {
+  // If there was no change, don't emit a remark.
----------------
Just pass by value?


================
Comment at: lib/IR/LegacyPassManager.cpp:155
+                               DiagnosticLocation(), &BB);
+  R << DiagnosticInfoOptimizationBase::Argument("Pass", P->getPassName())
+    << ": IR instruction count changed from "
----------------
DiagnosticInfoOptimizationBase::Argument -> NV (see many examples elsewhere).


================
Comment at: lib/IR/LegacyPassManager.cpp:163
+                                                FinalCount - OriginalCount);
+  F.getContext().diagnose(R);
+}
----------------
Add a comment that this is not using ORE for layering reasons.


https://reviews.llvm.org/D38768