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

[Nikita Popov via Phabricator via llvm-commits]

nikic 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);
----------------
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.


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:3320
+    // Unswitching guards/selects unswitches the entire loop.
+    if (isGuard(&TI) || isa<SelectInst>(TI))
+      return LoopCost;
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151677