[PATCH] D33567: [InstCombine] Pass the DominatorTree and AssumptionCache to a few calls to isKnownPositive, isKnownNegative, and isKnownNonZero

[Daniel Berlin via llvm-commits]

I tried to make sure we passed the right instruction in each case, but i
may have screwed up :)
note that:
 SQ.getWithInstruction(&I)
vs
 SQ will give different results if the *context instruction* is not I.

I swear there was a case in instcombine where this the case originally, and
i made sure it continues.

On Fri, May 26, 2017 at 10:06 AM, Sanjay Patel via Phabricator <
reviews at reviews.llvm.org> wrote:

> spatel added subscribers: davide, hfinkel, dberlin.
> spatel added a comment.
>
> Can you concoct tests with 'llvm.assume' to show a difference?
>
> I think that raises a question though: this change can make value tracking
> more effective, but it comes with non-zero compile-time cost if I
> understand computeKnownBits() correctly. Do we want to incur that overhead
> without a real-world motivation for the benefit?
> cc'ing @dberlin @davide @hfinkel
>
> Another question along this line: we call Simplify*Inst() for all
> instructions as the first step in every visit*() in InstCombine. In most
> cases, we don't pass a context instruction, but we do for visitICmpInst().
> Is that difference intentional?
>
>   if (Value *V = SimplifyICmpInst(I.getPredicate(), Op0, Op1,
>                                   SQ.getWithInstruction(&I)))
>     return replaceInstUsesWith(I, V);
>
> vs.
>
>   if (Value *V = SimplifyShlInst(Op0, Op1, I.hasNoSignedWrap(),
>                                   I.hasNoUnsignedWrap(), SQ))
>      return replaceInstUsesWith(I, V);
>
>
> https://reviews.llvm.org/D33567
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170526/7c07625a/attachment.html>