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

[Nikita Popov via Phabricator via llvm-commits]

nikic 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);
----------------
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.


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

https://reviews.llvm.org/D93973