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

[Serguei Katkov via Phabricator via llvm-commits]

skatkov added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/GuardWidening.cpp:576
+    // If there is no place to add freeze - return nullptr.
+    if (!Res || !DT.dominates(I, Res))
+      return nullptr;
----------------
mkazantsev wrote:
> How is this possible?
Invoke is in latch is used as a condition to go to header in normal path.
As a result getInsertionPointAfterDef() will return first non-phi instruction in header.

In this case I does not dominate Res. Actually it breaks SSA.


================
Comment at: llvm/lib/Transforms/Scalar/GuardWidening.cpp:582
+          Instruction *User = cast<Instruction>(U);
+          return Res != User && DT.dominates(I, User) &&
+                 !DT.dominates(Res, User);
----------------
mkazantsev wrote:
> `I` by default should dominate all its users. What am I missing here?
No, if the user is a phi node in header.


================
Comment at: llvm/lib/Transforms/Scalar/GuardWidening.cpp:611
+    Instruction *I = dyn_cast<Instruction>(V);
+    if (!I || canCreateUndefOrPoison(cast<Operator>(I),
+                                     /*ConsiderFlagsAndMetadata*/ false)) {
----------------
mkazantsev wrote:
> Why we can always cast this to operator? What if it's not?
Any Instruction can be casted to Operator.


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

https://reviews.llvm.org/D146699