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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

lgtm again



================
Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:1060
+    // We are only unswitching full LIV.
+    BranchesInfo.setUnswitched(SI, CondVal);
     ++NumSwitches;
----------------
I understand why you need this, but IIUC the `CurLoopInstructions` table is unnecessary.

We can solve the problem it solves in either of two ways:

 - Remove cases from switches after unswitching them.  That is, with `switch (V) { case 0: ... }` the `V != 0` version of the loop does not have a `case 0:`.  This would prevent us from unswitching on that condition again by definition.

 - Do not unswitch on cases that lead to a block that solely contains `unreachable`.  This way we will avoid doing redundant work generally (simplifycfg will clean these trivial cases up), and also prevent us unswitching `switch` es that we just unswitched.

So while your change itself is okay, depending on how much time you have it would be nice to remove `CurLoopInstructions` altogether.



https://reviews.llvm.org/D29107