[PATCH] D14122: [SimplifyCFG] Trim duplicate basic blocks in switch cases

[Philip Reames via llvm-commits]

If you decide to go forward with the prefix matching, SimplifyCFG is 
absolutely the right place.  I'm really bothered by the fact that target 
specific tests are sensitive to a target independent transform pass at 
all.  I didn't follow that part of the review, but any way we can fix 
that? Working around it further by introducing a new pass is definitely 
not something we should do.

Philip

On 10/28/2015 09:28 PM, Dylan McKay wrote:
> dylanmckay marked 2 inline comments as done.
> dylanmckay added a comment.
>
> Phillip's idea is a good one - it would be fairly easy to generalize this optimization so that it performes "prefix merging".
>
> Do you think the new optimization should still live in `SimplifyCFG`? One problem with keeping it in `SimplifyCFG` is that block merging will interfere with tests even worse than with this patch (which is why the current patch adds a command line option to disable merging), causing them to wonder "where has my basic block gone?".
>
> If the patch were to be split into a separate pass, it would be quite a small pass which would be overkill. Perhaps it would be better to keep the optimization in `SimplifyCFG` and keep the command line option for testing purposes.
>
> Thoughts?
>
>
> ================
> Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3461
> @@ -3418,3 +3460,3 @@
>     BasicBlock::iterator I = Succ->begin();
>     while (PHINode *PHI = dyn_cast<PHINode>(I++)) {
>       int Idx = PHI->getBasicBlockIndex(BB);
> ----------------
> I understand this now - thanks for the explanation Sanjoy!
>
>
> http://reviews.llvm.org/D14122
>
>
>