[PATCH] When canonicalizing gep indices, prefer zext when possible

[hfinkel at anl.gov]

================
Comment at: lib/Transforms/InstCombine/InstCombineCasts.cpp:1067-1074
@@ -1066,1 +1066,10 @@
 
+  // If we know that the value being extended is positive, we can use a zext
+  // instead. 
+  bool KnownZero, KnownOne;
+  ComputeSignBit(Src, KnownZero, KnownOne, 0, &CI);
+  if (KnownZero) {
+    Value *ZExt = Builder->CreateZExt(Src, DestTy);
+    return ReplaceInstUsesWith(CI, ZExt);
+  }
+
----------------
reames wrote:
> majnemer wrote:
> > reames wrote:
> > > majnemer wrote:
> > > > This looks like it can be a separate change.
> > > It could yes, but the broader question of whether this transform is a good idea is global to all the locations.  
> > > 
> > > When submitting, I'm happy to split this into a separate patch with it's own test.  
> > I'm of the opinion that zext should be more canonical than sext in this case because zext's are simpler to analyze.  I can't immediately see why this canonicalization wouldn't be preferable except for bugs/deficiencies in other combines.
> Ok, I'll clean this up and separate the two patches.  Do you want another round of review or should I just submit them?
I agree; I'm in favor of doing this.

http://reviews.llvm.org/D7255

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