[PATCH] D38802: [Dominators] Remove the NCA check

[Jakub (Kuba) Kuderski via llvm-commits]

>
> We could also avoid the other checks, and just build dominators with the
> simple iterative dataflow algorithm and see if it comes out the same.

Well, is there a high-level difference between that and the old
`DT.verifyDominatorTree`?

 The verification of tree properties is nice, though, as it guarantees
> correctness, while doing some other algorithm would just guarantee we maybe
> implemented it wrong twice :)

I think that we have to verify the properties, or at least some of them,
because we check that the DominatorTree is correct for the given CFG,
including the attributes we augmented it with (IE levels, DFS InOut
numbers). So for the validation to cover everything, we first have to check
that it has exactly the nodes that correspond to the CFG nodes
(reachability + roots), that the tree is the dominator tree (parent +
sibling), and that the extra information matches the tree (levels + DFS
numbers). If we switched solving dataflow equations, we would still have to
check the remaining properties.

On Wed, Oct 11, 2017 at 11:26 AM, Daniel Berlin <dberlin at dberlin.org> wrote:

> We could also avoid the other checks, and just build dominators with the
> simple iterative dataflow algorithm and see if it comes out the same.
> The verification of tree properties is nice, though, as it guarantees
> correctness, while doing some other algorithm would just guarantee we maybe
> implemented it wrong twice :)
>
>
> On Wed, Oct 11, 2017 at 8:24 AM, Brian Rzycki via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> brzycki accepted this revision.
>> brzycki added a comment.
>> This revision is now accepted and ready to land.
>>
>> When working on JumpThreading I never saw a failure due to NCD
>> verification so I think you're intuition about other checks makes sense.
>>
>>
>> https://reviews.llvm.org/D38802
>>
>>
>>
>>
>


-- 
Jakub Kuderski
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171011/50a4e5e7/attachment.html>