[llvm] r275244 - [ConstantFold] Don't incorrectly infer inbounds on array GEP

David Majnemer via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 12 20:24:41 PDT 2016


Author: majnemer
Date: Tue Jul 12 22:24:41 2016
New Revision: 275244

URL: http://llvm.org/viewvc/llvm-project?rev=275244&view=rev
Log:
[ConstantFold] Don't incorrectly infer inbounds on array GEP

The many levels of nesting inside the responsible code made it easy for
bugs to sneak in.  Flattening the logic makes it easier to see what's
going on.

Modified:
    llvm/trunk/lib/IR/ConstantFold.cpp
    llvm/trunk/test/Assembler/getelementptr.ll

Modified: llvm/trunk/lib/IR/ConstantFold.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/ConstantFold.cpp?rev=275244&r1=275243&r2=275244&view=diff
==============================================================================
--- llvm/trunk/lib/IR/ConstantFold.cpp (original)
+++ llvm/trunk/lib/IR/ConstantFold.cpp Tue Jul 12 22:24:41 2016
@@ -2192,51 +2192,68 @@ static Constant *ConstantFoldGetElementP
   bool Unknown = !isa<ConstantInt>(Idxs[0]);
   for (unsigned i = 1, e = Idxs.size(); i != e;
        Prev = Ty, Ty = cast<CompositeType>(Ty)->getTypeAtIndex(Idxs[i]), ++i) {
-    if (ConstantInt *CI = dyn_cast<ConstantInt>(Idxs[i])) {
-      if (isa<ArrayType>(Ty) && CI->getSExtValue() > 0 &&
-          !isIndexInRangeOfSequentialType(cast<ArrayType>(Ty), CI)) {
-        if (isa<SequentialType>(Prev)) {
-          // It's out of range, but we can factor it into the prior
-          // dimension.
-          NewIdxs.resize(Idxs.size());
-          uint64_t NumElements = 0;
-          if (auto *ATy = dyn_cast<ArrayType>(Ty))
-            NumElements = ATy->getNumElements();
-          else
-            NumElements = cast<VectorType>(Ty)->getNumElements();
-
-          ConstantInt *Factor = ConstantInt::get(CI->getType(), NumElements);
-          NewIdxs[i] = ConstantExpr::getSRem(CI, Factor);
-
-          Constant *PrevIdx = cast<Constant>(Idxs[i - 1]);
-          Constant *Div = ConstantExpr::getSDiv(CI, Factor);
-
-          unsigned CommonExtendedWidth =
-              std::max(PrevIdx->getType()->getIntegerBitWidth(),
-                       Div->getType()->getIntegerBitWidth());
-          CommonExtendedWidth = std::max(CommonExtendedWidth, 64U);
-
-          // Before adding, extend both operands to i64 to avoid
-          // overflow trouble.
-          if (!PrevIdx->getType()->isIntegerTy(CommonExtendedWidth))
-            PrevIdx = ConstantExpr::getSExt(
-                PrevIdx,
-                Type::getIntNTy(Div->getContext(), CommonExtendedWidth));
-          if (!Div->getType()->isIntegerTy(CommonExtendedWidth))
-            Div = ConstantExpr::getSExt(
-                Div, Type::getIntNTy(Div->getContext(), CommonExtendedWidth));
-
-          NewIdxs[i - 1] = ConstantExpr::getAdd(PrevIdx, Div);
-        } else {
-          // It's out of range, but the prior dimension is a struct
-          // so we can't do anything about it.
-          Unknown = true;
-        }
-        }
-    } else {
+    auto *CI = dyn_cast<ConstantInt>(Idxs[i]);
+    if (!CI) {
       // We don't know if it's in range or not.
       Unknown = true;
+      continue;
     }
+    if (isa<StructType>(Ty)) {
+      // The verify makes sure that GEPs into a struct are in range.
+      continue;
+    }
+    auto *STy = cast<SequentialType>(Ty);
+    if (isa<PointerType>(STy)) {
+      // We don't know if it's in range or not.
+      Unknown = true;
+      continue;
+    }
+    if (isa<VectorType>(STy)) {
+      // There can be awkward padding in after a non-power of two vector.
+      Unknown = true;
+      continue;
+    }
+    if (isIndexInRangeOfSequentialType(STy, CI))
+      // It's in range, skip to the next index.
+      continue;
+    if (!isa<SequentialType>(Prev)) {
+      // It's out of range, but the prior dimension is a struct
+      // so we can't do anything about it.
+      Unknown = true;
+      continue;
+    }
+    if (CI->getSExtValue() < 0) {
+      // It's out of range and negative, don't try to factor it.
+      Unknown = true;
+      continue;
+    }
+    // It's out of range, but we can factor it into the prior
+    // dimension.
+    NewIdxs.resize(Idxs.size());
+    // Determine the number of elements in our sequential type.
+    uint64_t NumElements = STy->getArrayNumElements();
+
+    ConstantInt *Factor = ConstantInt::get(CI->getType(), NumElements);
+    NewIdxs[i] = ConstantExpr::getSRem(CI, Factor);
+
+    Constant *PrevIdx = cast<Constant>(Idxs[i - 1]);
+    Constant *Div = ConstantExpr::getSDiv(CI, Factor);
+
+    unsigned CommonExtendedWidth =
+        std::max(PrevIdx->getType()->getIntegerBitWidth(),
+                 Div->getType()->getIntegerBitWidth());
+    CommonExtendedWidth = std::max(CommonExtendedWidth, 64U);
+
+    // Before adding, extend both operands to i64 to avoid
+    // overflow trouble.
+    if (!PrevIdx->getType()->isIntegerTy(CommonExtendedWidth))
+      PrevIdx = ConstantExpr::getSExt(
+          PrevIdx, Type::getIntNTy(Div->getContext(), CommonExtendedWidth));
+    if (!Div->getType()->isIntegerTy(CommonExtendedWidth))
+      Div = ConstantExpr::getSExt(
+          Div, Type::getIntNTy(Div->getContext(), CommonExtendedWidth));
+
+    NewIdxs[i - 1] = ConstantExpr::getAdd(PrevIdx, Div);
   }
 
   // If we did any factoring, start over with the adjusted indices.

Modified: llvm/trunk/test/Assembler/getelementptr.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Assembler/getelementptr.ll?rev=275244&r1=275243&r2=275244&view=diff
==============================================================================
--- llvm/trunk/test/Assembler/getelementptr.ll (original)
+++ llvm/trunk/test/Assembler/getelementptr.ll Tue Jul 12 22:24:41 2016
@@ -46,3 +46,13 @@ define <2 x i8*> @test8(<2 x [2 x i8]*>
   %w = getelementptr  [2 x i8], <2 x  [2 x i8]*> %a, <2 x i32> <i32 0, i32 0>, <2 x i8> <i8 0, i8 1>
   ret <2 x i8*> %w
 }
+
+ at array = internal global [16 x i32] [i32 -200, i32 -199, i32 -198, i32 -197, i32 -196, i32 -195, i32 -194, i32 -193, i32 -192, i32 -191, i32 -190, i32 -189, i32 -188, i32 -187, i32 -186, i32 -185], align 16
+
+; Verify that array GEP doesn't incorrectly infer inbounds.
+define i32* @test9() {
+entry:
+  ret i32* getelementptr ([16 x i32], [16 x i32]* @array, i64 0, i64 -13)
+; CHECK-LABEL: define i32* @test9(
+; CHECK: ret i32* getelementptr ([16 x i32], [16 x i32]* @array, i64 0, i64 -13)
+}




More information about the llvm-commits mailing list