[PATCH] D149345: [Utils] Added the ability to print the pass number and IR after it is triggered

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:109
 
+static cl::opt<bool> PrintPassNumbers(
+    "print-pass-numbers", cl::init(false), cl::Hidden,
----------------
so that we don't have an explosion of flag combinations, can we make it so this flag only takes effect when the other `-print` flags are on? i.e. this doesn't turn on printing on its own, rather it controls what gets printed if we are printing. this does require also passing whatever other flag (e.g. `-print-after-all` but I think that's fine. this should simplify a lot of the conditionals


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:114
+static cl::opt<unsigned>
+    PrintAfterPassNumber("print-after-pass-number", cl::init(0), cl::Hidden,
+                cl::desc("Print IR after pass with this number as "
----------------
this seems more like `print-at-pass-number` rather than `print-after-pass-number` IIUC


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:736
+  if (shouldPrintPassNumbers() || shouldPrintAfterPassNumber())
+    ++CurrentPassNumber;
+
----------------
might as well do this unconditionally


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:781
   SmallString<20> Banner =
       formatv("*** IR Dump After {0} on {1} (invalidated) ***", PassID, IRName);
+  if (shouldPrintAfterPassNumber())
----------------
put this in an `else`


================
Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:809
+bool PrintIRInstrumentation::shouldPrintPassNumbers() {
+  return PrintPassNumbers;
+}
----------------
isn't 0 a valid value? maybe make -1 the sentinel value


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149345