[PATCH] D46893: [CVP] Require DomTree for new Pass Manager

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 18 08:49:48 PDT 2018


dberlin accepted this revision.
dberlin added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D46893#1104202, @dmgreen wrote:

> I may have sowed some confusion here by not updating commit messages as the code changed.
>
> The code here in CVP currently:
>
> - use getBestSimplifyQuery to get the best SQ available (but not require a DT in the NPM)
> - use the SQ in SQ.DT->dominates (causing the problem) and SimplifyInstruction(.., SQ)


Yeah, this is a bad idea :)

> With this patch it:
> 
> - gets DT directly and passes it through to where it's needed (i.e DT->dominates)
> - uses getBestSimplifyQuery only for SimplifyInstruction(.., SQ)
> - no longer uses SQ.DT directly

This is a good idea ;)

> Everyone happy (enough) with this state of affairs?

I am for sure.

I'll mark this accepted, but please wait until monday to see if anyone has any more concerns.


https://reviews.llvm.org/D46893





More information about the llvm-commits mailing list