[PATCH] D30642: [ConstantFold] Fix defect in constant folding computation for GEP

[Javed Absar via Phabricator via llvm-commits]

javed.absar created this revision.

When the array indexes are all determined by GVN to be constants,
a call is made to constant-folding to optimize/simplify the address
computation. 
The constant-folding, however, makes a mistake in that it sometimes reads
back stale Idxs instead of NewIdxs, that it re-computed in previous iteration.
This leads to incorrect addresses coming out of constant-folding to GEP.

A test case is included. The error is only triggered when indexes have particular
patterns that the stale/new index updates interplay matters.


https://reviews.llvm.org/D30642

Files:
  lib/IR/ConstantFold.cpp
  test/Analysis/ConstantFolding/gep-constanfolding-error.ll


Index: test/Analysis/ConstantFolding/gep-constanfolding-error.ll
===================================================================
--- /dev/null
+++ test/Analysis/ConstantFolding/gep-constanfolding-error.ll
@@ -0,0 +1,52 @@
+; RUN: opt -gvn -S -o - %s | FileCheck %s
+; RUN: opt -newgvn -S -o - %s | FileCheck %s
+; Test that the constantfolding getelementptr computation results in
+; j[5][4][1] (j+239)
+; and not [1][4][4][1] (#449) which is an incorrect out-of-range error
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "armv7-none-eabi"
+
+ at f = local_unnamed_addr global i32 2, align 4
+ at t6 = local_unnamed_addr global i32 1, align 4
+ at j = local_unnamed_addr global [6 x [6 x [7 x i8]]] [[6 x [7 x i8]] [[7 x i8] c"\06\00\00\00\00\00\00", [7 x i8] zeroinitializer, [7 x i8] zeroinitializer, [7 x i8] zeroinitializer, [7 x i8] zeroinitializer, [7 x i8] zeroinitializer], [6 x [7 x i8]] zeroinitializer, [6 x [7 x i8]] zeroinitializer, [6 x [7 x i8]] zeroinitializer, [6 x [7 x i8]] zeroinitializer, [6 x [7 x i8]] zeroinitializer], align 1
+ at p = internal global i64 0, align 8
+ at y = local_unnamed_addr global i64* @p, align 4
+ at b = internal unnamed_addr global i32 0, align 4
+ at h = common local_unnamed_addr global i16 0, align 2
+ at a = common local_unnamed_addr global i32 0, align 4
+ at k = common local_unnamed_addr global i32 0, align 4
+ at t11 = common local_unnamed_addr global i32 0, align 4
+
+; Function Attrs: nounwind
+define i32 @main() local_unnamed_addr {
+entry:
+  %0 = load i32, i32* @t6, align 4
+  %inc = add nsw i32 %0, 1
+  store i32 %inc, i32* @t6, align 4
+  store i16 4, i16* @h, align 2
+  %1 = load i32, i32* @a, align 4
+  %conv = trunc i32 %1 to i8
+  store i32 1, i32* @f, align 4
+  %2 = load i64, i64* @p, align 8
+  %cmp4 = icmp slt i64 %2, 2
+  %conv6 = zext i1 %cmp4 to i8
+  %3 = load i16, i16* @h, align 2
+  %conv7 = sext i16 %3 to i32
+  %add = add nsw i32 %conv7, 1
+  %f.promoted = load i32, i32* @f, align 4
+  %4 = mul i32 %conv7, 7
+  %5 = add i32 %4, 5
+  %6 = sub i32 -1, %f.promoted
+  %7 = icmp sgt i32 %6, -2
+  %smax = select i1 %7, i32 %6, i32 -2
+  %8 = sub i32 6, %smax
+  %scevgep = getelementptr [6 x [6 x [7 x i8]]], [6 x [6 x [7 x i8]]]* @j, i32 0, i32 0, i32 %5, i32 %8
+  %9 = add i32 %f.promoted, %smax
+  %10 = add i32 %9, 2
+  call void @llvm.memset.p0i8.i32(i8* %scevgep, i8 %conv6, i32 %10, i32 1, i1 false)
+; CHECK:  call void @llvm.memset.p0i8.i32(i8* getelementptr inbounds ([6 x [6 x [7 x i8]]], [6 x [6 x [7 x i8]]]* @j, i32 0, i64 5, i64 4, i32 1), i8 %conv6, i32 1, i32 1, i1 false)
+; CHECK-NOT: call void @llvm.memset.p0i8.i32(i8* getelementptr ([6 x [6 x [7 x i8]]], [6 x [6 x [7 x i8]]]* @j, i64 1, i64 4, i64 4, i32 1)
+  ret i32 0
+}
+; Function Attrs: argmemonly nounwind
+declare void @llvm.memset.p0i8.i32(i8* nocapture writeonly, i8, i32, i32, i1)
Index: lib/IR/ConstantFold.cpp
===================================================================
--- lib/IR/ConstantFold.cpp
+++ lib/IR/ConstantFold.cpp
@@ -2181,6 +2181,7 @@
   // notional array or vector bounds. If so, try to determine if they can be
   // factored out into preceding dimensions.
   SmallVector<Constant *, 8> NewIdxs;
+  SmallVector<bool,8> IdxsChanged(Idxs.size(),false);
   Type *Ty = PointeeTy;
   Type *Prev = C->getType();
   bool Unknown = !isa<ConstantInt>(Idxs[0]);
@@ -2230,8 +2231,10 @@
 
     ConstantInt *Factor = ConstantInt::get(CI->getType(), NumElements);
     NewIdxs[i] = ConstantExpr::getSRem(CI, Factor);
+    IdxsChanged[i] = true;
 
-    Constant *PrevIdx = cast<Constant>(Idxs[i - 1]);
+    Constant *PrevIdx = (IdxsChanged[i-1]) ? NewIdxs[i-1] :
+                           cast<Constant>(Idxs[i - 1]);
     Constant *Div = ConstantExpr::getSDiv(CI, Factor);
 
     unsigned CommonExtendedWidth =
@@ -2249,6 +2252,7 @@
           Div, Type::getIntNTy(Div->getContext(), CommonExtendedWidth));
 
     NewIdxs[i - 1] = ConstantExpr::getAdd(PrevIdx, Div);
+    IdxsChanged[i-1] = true;
   }
 
   // If we did any factoring, start over with the adjusted indices.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30642.90673.patch
Type: text/x-patch
Size: 4076 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170306/0fa3c2bd/attachment.bin>