[PATCH] D96399: [X86][CodeGenPrepare] Try to reuse IV's incremented value instead of adding the offset

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

Max,

A couple of high level comments here.

1. You're code is specific to generating the address as GEPs.  I think we want to handle this for the non-GEP path as well.
2. I think this makes more sense to phrase as an optimization step for the AddrMode we're about to sink.  Doing it that way should also address (1) above.
3. I'm definitely not okay with the initial patch being so narrowly focused.  If you were handling generic add and sub, but not the overflow intrinsics I'd be less concerned, but the very narrow focus on one of the overflow along with your todo makes me strongly suspect there is a lurking correctness issue which the usub tests simply happen not to expose.
4. I think I might see the correctness issue, or at least a hint of it.  Consider the case where addressing does overflow.  The wrapping semantics of a GEP are not the same as the usubo.  That difference means that if overflow occurs, your optimized AddrMode is incorrect.  I believe you need to restrict this transform to when you can prove overflow causes the memory inst not to be reached.


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

https://reviews.llvm.org/D96399