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

[Adam Nemet via Phabricator via llvm-commits]

anemet accepted this revision.
anemet added a comment.
This revision is now accepted and ready to land.

Mostly nits.  LGTM with the requested changes.

(Nice test!)



================
Comment at: lib/IR/Function.cpp:198
+unsigned Function::getInstructionCount() {
+  int NumInstrs = 0;
+  for (BasicBlock &BB : BasicBlocks)
----------------
unsigned


================
Comment at: lib/IR/LegacyPassManager.cpp:151-167
+  // We need a function containing at least one basic block in order to output
+  // remarks. Since it's possible that the first function in the module doesn't
+  // actually contain a basic block, we have to go and find one that's suitable
+  // for emitting remarks.
+  Function *F = nullptr;
+  if (!F) {
+    auto It = std::find_if(M.begin(), M.end(),
----------------
Please make this logic more tight.  E.g. the initial check on F is a constant true and this last one is a constant false.




================
Comment at: test/Other/size-remarks.ll:10
+
+; For these remarks, the "function" field in the YAML file doesn't matter.
+; Each of the testcases work by combining the output remarks with the
----------------
Can you please file a bugzilla that we should support remarks with modules as the code region (Product:tools, Component:opt-viewer).

The tricky part is that Function is pretty deeply baked into the remarks so this may need some work.


https://reviews.llvm.org/D38768