Answer to Philip Reames comments

[oota-llvm.git] / lib / Transforms / Scalar / SeparateConstOffsetFromGEP.cpp

diff --git a/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
index 3f84ffde9464195c735e074f510ab4ee7c678c04..6557ce4575dda2995ea8b7d885bc543160e16e47 100644 (file)
--- a/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
+++ b/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
@@ -268,6 +268,8 @@ class SeparateConstOffsetFromGEP : public FunctionPass {
   /// This canonicalization is very likely already done in clang and
   /// instcombine. Therefore, the program will probably remain the same.
   ///
+  /// Returns true if the module changes.
+  ///
   /// Verified in @i32_add in split-gep.ll
   bool canonicalizeArrayIndicesToPointerSize(GetElementPtrInst *GEP);
 
@@ -321,9 +323,9 @@ bool ConstantOffsetExtractor::CanTraceInto(bool SignExtended,
   //       1       |      0       | sext(BO) == sext(A) op sext(B)
   //       1       |      1       | zext(sext(BO)) ==
   //               |              |     zext(sext(A)) op zext(sext(B))
-  if (BO->getOpcode() == Instruction::Add && NonNegative) {
+  if (BO->getOpcode() == Instruction::Add && !ZeroExtended && NonNegative) {
     // If a + b >= 0 and (a >= 0 or b >= 0), then
-    //   s/zext(a + b) = s/zext(a) + s/zext(b)
+    //   sext(a + b) = sext(a) + sext(b)
     // even if the addition is not marked nsw.
     //
     // Leveraging this invarient, we can trace into an sext'ed inbound GEP
@@ -403,7 +405,6 @@ APInt ConstantOffsetExtractor::find(Value *V, bool SignExtended,
     // sext(zext(a)) = zext(a). Verified in @sext_zext in split-gep.ll.
     //
     // Clear the NonNegative flag, because zext(a) >= 0 does not imply a >= 0.
-    // TODO: if zext(a) < 2 ^ (bitwidth(a) - 1), we can prove a >= 0.
     ConstantOffset =
         find(U->getOperand(0), /* SignExtended */ false,
              /* ZeroExtended */ true, /* NonNegative */ false).zext(BitWidth);
@@ -629,8 +630,7 @@ bool SeparateConstOffsetFromGEP::splitGEP(GetElementPtrInst *GEP) {
   if (GEP->hasAllConstantIndices())
     return false;
 
-  bool Changed = false;
-  Changed |= canonicalizeArrayIndicesToPointerSize(GEP);
+  bool Changed = canonicalizeArrayIndicesToPointerSize(GEP);
 
   bool NeedsExtraction;
   int64_t AccumulativeByteOffset = accumulateByteOffset(GEP, NeedsExtraction);