[PATCH] D14122: [SimplifyCFG] Trim duplicate basic blocks in switch cases
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 28 21:39:14 PDT 2015
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
>
>
>
More information about the llvm-commits
mailing list