[PATCH] [BasicAA] Fix zext & sext handling

[Nick White]

Hi @sanjoy - I've revised the patch to incorporate your suggestions. Is it OK now? Thanks -

Nick


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:275
@@ +274,3 @@
+      if (isa<OverflowingBinaryOperator>(BOp)) {
+        NUW &= BOp->hasNoUnsignedWrap();
+        NSW &= BOp->hasNoSignedWrap();
----------------
sanjoy wrote:
> Note that currently in the LangRef, `shl nsw X, Y` is not the same as `mul nsw X (1 << Y)`, the LangRef has a bug in this regard.  David has an out of tree patch to fix this http://reviews.llvm.org/D8890 but meanwhile you might want to not propagate `nuw` and `nsw` for shifts.
That's interesting - I've added an early escape for the Shl that sets NSW & NUW to false in the latest version of this patch so they won't be propagated.

================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:300
@@ +299,3 @@
+    // we'll only execute the if-true block if we sext'ing a value that hasn't
+    // been zext'ed (i.e. we'll actually sign extend the variable, instead of
+    // converting the sext to a zext).
----------------
sanjoy wrote:
> Very minor, but I think it will be clearer if you just add an annotation to each branch of the if statement:
> 
> ```
> if (is<SEXtInst>(V) && ZExtBits == 0) {
>   // sext(sext(A)) == sext(A)
> ```
> 
> etc. instead of a comprehensive meta comment.
I've changed this in my latest revision of this patch.

http://reviews.llvm.org/D6682

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