[PATCH] D29107: Fix a bug when unswitching on partial LIV for SwitchInst

[Xin Tong via Phabricator via llvm-commits]

trentxintong added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:439
+          if (Value *LHS =
+                  FindLIVLoopCondition(BO->getOperand(0), L, Changed, OCS, Cache)) {
+            Cache[Cond] = LHS;
----------------
sanjoy wrote:
> Doesn't this regress the callers of `FindLIVLoopCondition` that pass in `nullptr` for `OCS`?  They used to get the recursive search before, and now they don't?
> 
> My preference would be to avoid allowing a null `OCS`, and instead have it be a `OperatorChainTy &` instead.  Callers that don't need it can pass in a dummy.
The only places which we pass in a nullptr is TryTrivialLoopUnswitch (with the patch), i.e. before the patch TryTrivialLoopUnswitch ignores partial LIV even we manage to get one by walking up the chain. So there is no place which will get regressed in the current state the pass is written. 




https://reviews.llvm.org/D29107