[PATCH] D48763: [SimplifyIndVar] Canonicalize comparisons to unsigned while eliminating truncs

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyIndVar.cpp:558
 
+  auto HighestBitsMatch = [&](Value *X, Value *Y) {
+    const SCEV *SCEVX = SE->getSCEV(X);
----------------
greened wrote:
> greened wrote:
> > That name seems misleading.  It's not checking if they match, it's checking if the higest bit is zero for both.  Did you mean to add a check for both being negative as well?  The comments below seem to indicate so.
> Why does this need to be a lambda?
Yes, actually in the initial version it also checked negative, but then I realized that for negative values `zext(trunc(x))` is never equal to `x`, so in practice this case will never fire. I will change the naming here.


================
Comment at: lib/Transforms/Utils/SimplifyIndVar.cpp:563
+  };
+  auto CanUseZExt = [&](ICmpInst *ICI) {
+    // Unsigned comparison can be widened as unsigned.
----------------
greened wrote:
> Does this need to be a lambda?  Seems unnecessarily complicated.
It is the simplest way to avoid something like `if (Cond1 || (!Cond2 && Cond3 && Cond4))` which is harder to understand.


https://reviews.llvm.org/D48763