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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyIndVar.cpp:340
+                                   const SCEV *SD) {
+  if (!SE->isKnownNonNegative(SN) || !SE->isKnownNonNegative(SD))
+    return false;
----------------
etherzhhb wrote:
> sanjoy wrote:
> > Can we share this work amongst the three helpers?  Something like:
> > 
> >  - Pass in `NumeratorNonNegative` and `DenominatorNonNegative` bools to this function, and only `NumeratorNonNegative` to the other two.
> >  - Compute these bools in `simplifyIVRemainder` as needed
> Sure, I will do this. I think this whole simplifyIVRemainder will not process if NumeratorNonNegative  is false, is that correct?
Yes, but I think it may be cleaner to pass it in to the helpers; that way the dependency of the transform is clearer.  And just returning from the helpers if it isn't set seems fine.


Repository:
  rL LLVM

https://reviews.llvm.org/D38072