[PATCH] D26998: [StructurizeCFG] Refactor NearestCommonDominator.

Justin Lebar via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 22 13:47:32 PST 2016


jlebar created this revision.
jlebar added a reviewer: arsenm.
jlebar added a subscriber: llvm-commits.
Herald added subscribers: wdng, aemerson.

As far as I can tell, doing our own computations in
NearestCommonDominator is a false optimization -- DomTree will build up
what appears to be exactly this data when it decides it's worthwhile.
Moreover, by building the cache ourselves, we cannot take advantage of
the cache that the domtree might have available.

In addition, I am not convinced of the correctness of the original code.
In particular, setting ResultIndex = 1 on the first addBlock instead of
setting it to 0 is quite fishy.  Similarly, it's not clear to me that
setting IndexMap[Node] = 0 for every node as we walk up the tree finding
a common parent is correct.  But rather than ponder over these
questions, I'd rather just make the code do the obviously-correct thing.

This patch also changes the NearestCommonDominator API a bit, improving
the names and getting rid of the boolean parameter in addBlock -- see
http://jlebar.com/2011/12/16/Boolean_parameters_to_API_functions_considered_harmful..html


https://reviews.llvm.org/D26998

Files:
  llvm/lib/Transforms/Scalar/StructurizeCFG.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26998.78943.patch
Type: text/x-patch
Size: 5227 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161122/92b9aad4/attachment.bin>


More information about the llvm-commits mailing list