[PATCH] PR17473: Disallow LSR to perform non-invertible normalization

[Arnold Schwaighofer]

The code LGTM. Maybe we add a comment like:

+      // PostIncNormalization effectively simplifies the expression under
+      // pre-increment assumptions. Those assumptions (no wrapping) might not
+      // hold for the post-inc value. Catch such cases by making sure the
+      // transformation is invertible.
+      if (OriginalISE != ISE) {


Can you remove the definition of the function “fn3” it is already inlined in your test case so you don’t need the definition?

The test case can be further simplified by removing TBAA tags like ", !tbaa !1” and removing the metadata at the end:

+!llvm.ident = !{!0}
+
+!0 = metadata !{metadata !"clang version 3.5.0 "}
+!1 = metadata !{metadata !2, metadata !2, i64 0}
+!2 = metadata !{metadata !"int", metadata !3, i64 0}
+!3 = metadata !{metadata !"omnipotent char", metadata !4, i64 0}
+!4 = metadata !{metadata !"Simple C/C++ TBAA"}
+!5 = metadata !{metadata !3, metadata !3, i64 0}


Thanks,
Arnold

On Mar 10, 2014, at 2:56 PM, Michael Zolotukhin <mzolotukhin at apple.com> wrote:

> Hi,
> 
> This is a fix for PR17473. The issue is that LSR performs normalization of detected IV users and sometimes wants to denormalize it back hoping to get the original expression. But normalization isn’t always invertible, and the new expression might be not equivalent to the original one. In the test case, we were losing sign-extension in such transformation. The patch simply adds a check, if the transformation is invertible. 
> 
> <pr17473.patch>
> 
> Ok for trunk?
> 
> Thanks,
> Michael