[PATCH] D31239: [WIP] Add Caching of Known Bits in InstCombine

[Daniel Berlin via llvm-commits]

In which i repeat my claim that trying to keep caches up to date is
significantly harder to get right and verify than incremental recomputation
:)
 I additionally claim that we should actually quantify the performance
benefits of doing either over "recomputing once or twice during pipeline"
before going down this route.

We are *already* trading off compile time performance vs generated code
performance.

All other things being equal, we should trade it off in a way that makes
maintenance of code and verification of correctness as easy as possible.

Asserting that a good tradeoff to reduce compile time is "cache" instead of
"stop doing it on demand" (or incrementally recompute), without any data
other than compile time performance seems ... not particularly scientific.

It's certainly true that caching is often posited as "easier", but i think
a trip through bugzilla would put this idea to rest.



On Fri, Mar 24, 2017 at 3:33 PM, Craig Topper via Phabricator <
reviews at reviews.llvm.org> wrote:

> craig.topper added a comment.
>
> I'm seeing more problems than just nsw/nuw flags here.
>
> sext.ll test is failing because SimplifyDemandedInstructions bits
> determined that this
>
> %and = and i32 %x, 16
>
>   shl i32 %and, 27
>
> Simplified to just the shl because we were only demanding the MSB of the
> shift. This occurred after we had cached the known bits for the shl as
> having 31 lsbs as 0. But without the "and" in there we can no longer
> guarantee the lower bits of the shift result are 0.
>
> I also got a failure on shift.ll not reported here. This was caused by
> getShiftedValue rewriting operands and changing constants of other shifts.
> This effectively shifts the Known bits and thus the cache needs to be
> invalidate.
>
>
> https://reviews.llvm.org/D31239
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170324/e3e14233/attachment.html>