[PATCH] D41299: [PDT] Fix splitBlock for Post Dom Trees

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 19 12:12:56 PST 2017


fhahn added a comment.

In https://reviews.llvm.org/D41299#959733, @dmgreen wrote:

> Hello, Thanks for taking a look.
>
> I went looking for tests for splitBlock on DomTrees and was surprised not to find anything. I guess it gets tested as part of preserving trees in normal IR tests. I will see if I can add something sensible.


PostDominators are not really used extensively, so relying on the IR tests here is not the best idea. `unittests/IR/DominatorTreeTest.cpp` and/or `unittests/IR/DominatorTreeBatchUpdatesTest.cpp` should be good places to add a unit test that fails without this patch. I assume you have a case where you discovered the failure?

> I'm not 100% sure yet, but making this work with the updater from d40146 might need some changes. I will try it to see what the best way to handle these split blocks might be with that interface.

I think the reason `splitBlock` is there is because it's cheap to update the DomTree in this case. If we have to use the `applyUpdates`, it might be better to batch those changes with all other DomTree changes.


https://reviews.llvm.org/D41299





More information about the llvm-commits mailing list