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

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 26 21:12:22 PST 2017

sanjoy added a comment.

lgtm again

Comment at: lib/Transforms/Scalar/LoopUnswitch.cpp:1060
+    // We are only unswitching full LIV.
+    BranchesInfo.setUnswitched(SI, CondVal);
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.


More information about the llvm-commits mailing list