[PATCH] Remove loop variant range check when induction variable is strictly increasing

[Andrew Trick]

This is great.

I will say that it's not very nice to call SCEVExpander within the SimplifyIndvar utility on an arbitrary SCEV value. We can't control what SCEVExpander will do outside the loop. I think the SimplifyIndvars utility should be limited to folding instructions within the loop, and possibly cloning an IV increment, but not inserting code outside the loop. I can't think of any value in doing this inside SimplifyIndvar--it won't expose other simplification. I do agree that this optimization belongs in the indvars pass, but it could probably be a separate sub-pass over the loop's terminators.

As always, I'm nervous about using SCExpander in general. I'm not sure if we should have any checks for whether the start value can be safely or profitably materialized (see the terrible isSafeToExpand hack). It may be ok since we similary expand AddRecs and start values in WidenIV and LoopVectorize, but I can't say for sure. Just as an contrasting example implementation, if you wanted to be very conservative, you could find an existing loop phi whose initial value is your desired AddRec start with a constant offset. Then you can ask SCEVExpander to materialize that existing value wrapped in SCEVUnknown plus the offset (always safe).

At any rate, when using SCEVExpander it should be explicit and obvious at the top-level of the pass, not buried in a utility that can be called anywhere. That way it's easier to see that almost arbitrary IR rewriting may happen at that point (hopefully it's at least limited to adding new instructions, but without bounding the expression it can do some surprising things).


http://reviews.llvm.org/D9784

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/