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

[Sanjay Patel via llvm-commits]

On Fri, May 26, 2017 at 11:13 AM, Daniel Berlin <dberlin at dberlin.org> wrote:

> 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.
>
>
Is that because we only use CxtI in icmp-related queries like
computeKnownBitsFromAssume?
Ie, when we call Simplify*Inst with the plain 'SQ' in instcombine, CxtI is
'null' in that query. There is nothing filling that in with the default 'I'
from what I see.

If the simplify routine uses computeKnownBits or some other value tracking,
then there could be a different result if known bits was somehow using CxtI
for non-icmp analysis?



> 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/569b5140/attachment.html>