[PATCH] D151677: [SimpleLoopUnswitch] Unswitch AND/OR conditions of selects

[Joshua Cao via Phabricator via llvm-commits]

caojoshua marked 2 inline comments as done.
caojoshua added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:2875
         // restrict to simple boolean selects
-        if (!isa<Constant>(Cond) && L.isLoopInvariant(Cond) && Cond->getType()->isIntegerTy(1))
-          UnswitchCandidates.push_back({&I, {Cond}});
+        if (Cond->getType()->isIntegerTy(1))
+          AddUnswitchCandidatesForInst(SI, Cond);
----------------
nikic wrote:
> I think we should also add a `&& !SI->getType()->isIntegerTy(1)` check here to ensure this does not unswitch part of a logical and/or, as these should be handled by other code.
Yes, I agree. End result in default O3 pipeline is the same, but we have more in pass simplifications if we use and/or branch unswitches.


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:3320
+    // Unswitching guards/selects unswitches the entire loop.
+    if (isGuard(&TI) || isa<SelectInst>(TI))
+      return LoopCost;
----------------
nikic wrote:
> Hm, why do guards need to be handled the same ways as selects? Don't we effectively save the code dominated by the guard in one of the loops?
Yes, that's a mistake. Only doing this for selects now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151677