[PATCH] D146699: [GuardWidening] Freeze the introduced use.

[Serguei Katkov via Phabricator via llvm-commits]

skatkov marked an inline comment as done.
skatkov added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/GuardWidening.cpp:569
+// Return Instruction before which we can insert freeze for the value V.
+// If V is an instruction then next instruction is returned. Otherwise first
+// instruction in entry basic block is returned.
----------------
mkazantsev wrote:
> This is not true for Phis
Doc is re-frased.


================
Comment at: llvm/lib/Transforms/Scalar/GuardWidening.cpp:570
+// If V is an instruction then next instruction is returned. Otherwise first
+// instruction in entry basic block is returned.
+// If there is no place to add freeze, return nullptr.
----------------
mkazantsev wrote:
> non-alloca? Better say `first available insertion point in the entry block...`
Doc is re-frased.


================
Comment at: llvm/lib/Transforms/Scalar/GuardWidening.cpp:572
+// If there is no place to add freeze, return nullptr.
+static Instruction *getFreezeInsertPt(Value *V, DominatorTree &DT) {
+  if (auto *I = dyn_cast<Instruction>(V)) {
----------------
mkazantsev wrote:
> could be `const DominatorTree &DT`
Changed.


================
Comment at: llvm/lib/Transforms/Scalar/GuardWidening.cpp:588
+  }
+  return &*DT.getRoot()->getFirstNonPHIOrDbgOrAlloca();
+}
----------------
mkazantsev wrote:
> Maybe to reduce nest of code above, do
> ```
> auto *I = dyn_cast<Instruction>(V);
> if (!I)
>   return &*DT.getRoot()->getFirstNonPHIOrDbgOrAlloca();
> // code for instruction with 1 less nest
> ```
Done


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

https://reviews.llvm.org/D146699