[PATCH] D38072: [SimplifyIndvar] Replace the srem used by IV if we can prove both of its operands are non-negative

[Hongbin Zheng via Phabricator via llvm-commits]

etherzhhb added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyIndVar.cpp:401
+  replaceSRemWithURem(Rem);
+  // Only propagate the change if IVOperand is used as numerator
+  Changed = UsedAsNumerator;
----------------
sanjoy wrote:
> I'm not sure if this is safe -- it seems like `Changed` is piped all the way through to `IndVarSimplify`'s  return value to the pass manager (if nothing changed, it will tell the PM that all analyses are preserved).
> 
> Even if this is safe, I suspect you'll need to do `Changed |= UsedAsNumerator`.  In this case, please add a one liner on why this is safe.
> 
Thanks for you reminder on this, I have a feeling that this may not be safe as well. Let me have a closer look


Repository:
  rL LLVM

https://reviews.llvm.org/D38072