[llvm] 7ef2c68 - [InstSimplify] improve efficiency for detecting non-zero value

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 29 13:28:29 PDT 2021


Aren't disappearing bugs fun?  :)

Skimming the thread, one thing we might want to consider is teaching the 
vectorizer how to widen an assume.  This would attack the problem from 
another angle.

For any assume(X), if we unroll that as:
assume(X1)
assume(X2)
....
assume(XN)

We could also widen that into a single assume as:
%vec = {X1, X2, ..., XN)
%c = horizontal_and(%vec)
assume(%c)

The catch is that I don't think we've considered whether we want this to 
be a canonical form or not.  For the vectorizer with %vec already 
existing, I think it makes good sense though.

Philip

On 4/29/21 12:42 PM, Sanjay Patel wrote:
> Ah - the degenerate case was based on:
> https://bugs.llvm.org/show_bug.cgi?id=49785#c2 
> <https://bugs.llvm.org/show_bug.cgi?id=49785#c2>
>
> But I just reverted this locally:
> https://reviews.llvm.org/rGbb907b26e2bf2a0d5ea3fcb13ba0ccef3420a38c 
> <https://reviews.llvm.org/rGbb907b26e2bf2a0d5ea3fcb13ba0ccef3420a38c>
>
> And neither the original C example from PR49785 nor the IR reduction 
> are taking forever to compile for me. Something else must have changed 
> in the past 2 weeks to mask the problem. So I'm not sure how to 
> trigger this again.
>
> On Wed, Apr 28, 2021 at 5:19 PM Philip Reames 
> <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
>
>     Sanjay,
>
>     Neither of these seem to be the degenerate test case you saw the
>     compile time impact on.  Could you point me to that?  I'm looking
>     for something to run to understand the compile time opportunities
>     in this code.
>
>     Philip
>
>     On 4/15/21 6:48 AM, Sanjay Patel wrote:
>>     I'm not understanding the question. 🤔
>>
>>     I suspect this doesn't answer it, but more of the trail that led
>>     here:
>>     There was a gap where isKnownNonEqual was succeeding where
>>     simplifyICmpWithZero() did not - it was plugged with:
>>     https://reviews.llvm.org/rG5ae5d25e38efad1d59ed97d969a5e930b58a5e16
>>     <https://reviews.llvm.org/rG5ae5d25e38efad1d59ed97d969a5e930b58a5e16>
>>
>>     That was exposed by this test (and that's what the unittest in
>>     the above commit is based on):
>>     https://github.com/llvm/llvm-project/blob/9345f9fa5d6401bf9b98ce915bf4fbde9d183ce7/llvm/test/Analysis/ValueTracking/monotonic-phi.ll#L185
>>     <https://github.com/llvm/llvm-project/blob/9345f9fa5d6401bf9b98ce915bf4fbde9d183ce7/llvm/test/Analysis/ValueTracking/monotonic-phi.ll#L185>
>>
>>
>>     On Wed, Apr 14, 2021 at 6:04 PM Philip Reames
>>     <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
>>
>>         Hm, this seems interesting.
>>
>>         I had been thinking about inverting this API by having the
>>         shared code
>>         return the two pointers which imply equality/inequality and
>>         having
>>         instcombine rewrite the condition.  However, I couldn't find
>>         a case
>>         where the existing logic didn't already do that inversion
>>         through some
>>         other (more general) path.  Do you happen to have an IR
>>         fragment where
>>         we trip this example?  I'd like to take a look at it with
>>         that lens.
>>
>>         Philip
>>
>>         On 4/14/21 6:12 AM, Sanjay Patel via llvm-commits wrote:
>>         > Author: Sanjay Patel
>>         > Date: 2021-04-14T09:04:15-04:00
>>         > New Revision: 7ef2c68a3d24af0b0d540e748e8b564180f4e18a
>>         >
>>         > URL:
>>         https://github.com/llvm/llvm-project/commit/7ef2c68a3d24af0b0d540e748e8b564180f4e18a
>>         <https://github.com/llvm/llvm-project/commit/7ef2c68a3d24af0b0d540e748e8b564180f4e18a>
>>         > DIFF:
>>         https://github.com/llvm/llvm-project/commit/7ef2c68a3d24af0b0d540e748e8b564180f4e18a.diff
>>         <https://github.com/llvm/llvm-project/commit/7ef2c68a3d24af0b0d540e748e8b564180f4e18a.diff>
>>         >
>>         > LOG: [InstSimplify] improve efficiency for detecting
>>         non-zero value
>>         >
>>         > Stepping through callstacks in the example from D99759 reveals
>>         > this potential compile-time improvement.
>>         >
>>         > The savings come from avoiding ValueTracking's computing known
>>         > bits if we have already dealt with special-case patterns.
>>         >
>>         > Further improvements in this direction seem possible.
>>         >
>>         > This makes a degenerate test based on PR49785 about 40x faster
>>         > (25 sec -> 0.6 sec), but it does not address the larger
>>         question
>>         > of how to limit computeKnownBitsFromAssume(). Ie, the original
>>         > test there is still infinite-time for all practical purposes.
>>         >
>>         > Differential Revision: https://reviews.llvm.org/D100408
>>         <https://reviews.llvm.org/D100408>
>>         >
>>         > Added:
>>         >
>>         >
>>         > Modified:
>>         >      llvm/lib/Analysis/InstructionSimplify.cpp
>>         >
>>         > Removed:
>>         >
>>         >
>>         >
>>         >
>>         ################################################################################
>>         > diff  --git a/llvm/lib/Analysis/InstructionSimplify.cpp
>>         b/llvm/lib/Analysis/InstructionSimplify.cpp
>>         > index b233a0f3eb2d..08f504a0ce37 100644
>>         > --- a/llvm/lib/Analysis/InstructionSimplify.cpp
>>         > +++ b/llvm/lib/Analysis/InstructionSimplify.cpp
>>         > @@ -3432,6 +3432,8 @@ static Value
>>         *SimplifyICmpInst(unsigned Predicate, Value *LHS, Value *RHS,
>>         >     if (Value *V = simplifyICmpOfBools(Pred, LHS, RHS, Q))
>>         >       return V;
>>         >
>>         > +  // TODO: Sink/common this with other potentially
>>         expensive calls that use
>>         > +  //       ValueTracking? See comment below for
>>         isKnownNonEqual().
>>         >     if (Value *V = simplifyICmpWithZero(Pred, LHS, RHS, Q))
>>         >       return V;
>>         >
>>         > @@ -3637,7 +3639,9 @@ static Value
>>         *SimplifyICmpInst(unsigned Predicate, Value *LHS, Value *RHS,
>>         >     }
>>         >
>>         >     // icmp eq|ne X, Y -> false|true if X != Y
>>         > -  if (ICmpInst::isEquality(Pred) &&
>>         > +  // This is potentially expensive, and we have already
>>         computedKnownBits for
>>         > +  // compares with 0 above here, so only try this for a
>>         non-zero compare.
>>         > +  if (ICmpInst::isEquality(Pred) && !match(RHS, m_Zero()) &&
>>         >         isKnownNonEqual(LHS, RHS, Q.DL, Q.AC <http://Q.AC>,
>>         Q.CxtI, Q.DT, Q.IIQ.UseInstrInfo)) {
>>         >       return Pred == ICmpInst::ICMP_NE ? getTrue(ITy) :
>>         getFalse(ITy);
>>         >     }
>>         >
>>         >
>>         >
>>         > _______________________________________________
>>         > llvm-commits mailing list
>>         > llvm-commits at lists.llvm.org
>>         <mailto:llvm-commits at lists.llvm.org>
>>         >
>>         https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>         <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210429/997f520a/attachment.html>


More information about the llvm-commits mailing list