[PATCH] D47522: [PM/LoopUnswitch] Add partial non-trivial unswitching for invariant conditions feeding a chain of `and`s or `or`s for a branch.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Thanks for the review! Based on LGTM, I'll land once the dependent patch lands. Give a shout if you still have concerns though.



================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:162
+       make_range(std::next(Invariants.begin()), Invariants.end()))
+    if (Direction)
+      Cond = IRB.CreateOr(Cond, Invariant);
----------------
asbirlea wrote:
> Nit (feel free to ignore): I would find clearer the intention if this was written as:
> ```
> if (Direction)
>    for all invariants
>         Cond = CreateOr(Cond, Inv)
> else
>    for all invariants
>         Cond = CreateAnd(Cond, Inv)
> ```
> ( or are you relying on this being unswitched in bootstrap? :-) )
> 
The reason I don't want to do this is because it requires the for loop itself to be duplicated which has some logic in it, so I'd rather keep this factoring.

(But no, I wasn't just *trying* to exercise the self)


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:1814
+      else if (DT.dominates(ClonedPH, UserI->getParent()))
+        U->set(UnswitchedReplacement);
+    }
----------------
asbirlea wrote:
> Is it possible for neither LoopPH or ClonedPH to dominate the invariant use?
Yeah. Consider a user completely outside of the loop, for example some random other use of a function argument.


Repository:
  rL LLVM

https://reviews.llvm.org/D47522