[PATCH] D93973: [SCEV] Simplify trunc to zero based on known bits

[Gil Rapaport via Phabricator via llvm-commits]

gilr added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:1181
+    // desired recursion depth, only do this for unknown SCEV's.
+    if (const SCEVUnknown *U = dyn_cast<SCEVUnknown>(Op)) {
+      uint32_t MinTrailingZeros = GetMinTrailingZeros(Op);
----------------
nikic wrote:
> fhahn wrote:
> > Are you worried about the cost of calling `GetMintrailingZeros` here? Could it happen that we have a arbitrary SCEV expression here for which we can get more accurate results thanks to `GetMinTrailingZeros`?
> I believe this optimization can only be useful in the first place for SCEVUnknown (and SCEVPtrToInt, which is basically the same thing), as well as min/max expression. The latter only because we don't push truncates through min/max like we do for everything else. Possibly we should be doing that, but that's a different issue...
> 
> That said, from a code structure perspective, I'm not sure why this check is present in the "Depth > MaxCastDepth" branch at all. This is a recursion cut-off that is //supposed// to produce sub-optimal SCEV expressions, and there does not seem be any strong cause why this particular optimization needs to be applied unconditionally.
>I believe this optimization can only be useful in the first place for SCEVUnknown

This patch was indeed motivated by assumptions, but the analysis first needs to get to the SCEVUnknowns. Calling GetMinTrailingZeros on any SCEV that got through GetTruncateExpr complements what ever the latter didn't simplify, but admittedly GetMinTrailingZeros is unboundedly recursive itself (even if cached and unrelated to getTruncateExpr's recursion).

>This is a recursion cut-off that is supposed to produce sub-optimal SCEV expressions

Cutting off potentially exponential recursion at some point makes a lot of sense. Not sure it implies not doing any work at the leaves though.

To be on the safe side let's start by calling GetMinTrailingZeros only for SCEVUnknowns at the end of the function and extend as needed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93973/new/

https://reviews.llvm.org/D93973