[PATCH] D23727: [Profile] SelectInst instrumentation Support in IR-PGO

[David Li via llvm-commits]

davidxl marked 4 inline comments as done.

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:183
@@ +182,3 @@
+  // Visit \p SI instruction and perform tasks according to visit mode.
+  void visitSelectInst(SelectInst &SI);
+  unsigned size() const { return NSIs; }
----------------
vsk wrote:
> override?
It is not supposed to be virtual.

================
Comment at: lib/Transforms/Instrumentation/PGOInstrumentation.cpp:978
@@ +977,3 @@
+  }
+}
+
----------------
vsk wrote:
> My impression is that this could be better structured / easier to read as two different InstVisitors. Both visitors would maintain less state than the current SelectInstVisitor, and their constructors should obviate the need SetCounterIndex and SetCounterUse.
The instrumentation and use need to traverse IR in exactly the same way. Even though splitting it into two visitors does not change that, I'd like to keep them using the same class to show this strong tie.


https://reviews.llvm.org/D23727