From 8f0a1cecc582145903a2bc0d0e53f06f874fc831 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Fri, 19 Jul 2013 07:12:23 +0000 Subject: [PATCH] Fix PR16651, an assert introduced in my recent re-work of the innards of SROA. The crux of the issue is that now we track uses of a partition of the alloca in two places: the iterators over the partitioning uses and the previously collected split uses vector. We weren't accounting for the fact that the split uses might invalidate integer widening in ways other than due to their width (in this case due to being volatile). Further reduced testcase added to the tests. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@186655 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/Scalar/SROA.cpp | 12 +++--------- test/Transforms/SROA/basictest.ll | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/Transforms/Scalar/SROA.cpp b/lib/Transforms/Scalar/SROA.cpp index 41e96d625a6..91d5240eb59 100644 --- a/lib/Transforms/Scalar/SROA.cpp +++ b/lib/Transforms/Scalar/SROA.cpp @@ -1694,20 +1694,14 @@ isIntegerWideningViable(const DataLayout &TD, Type *AllocaTy, !canConvertValue(TD, IntTy, AllocaTy)) return false; - // If we have no actual uses of this partition, we're forming a fully - // splittable partition. Assume all the operations are easy to widen (they - // are if they're splittable), and just check that it's a good idea to form - // a single integer. - if (I == E) - return TD.isLegalInteger(SizeInBits); - uint64_t Size = TD.getTypeStoreSize(AllocaTy); // While examining uses, we ensure that the alloca has a covering load or // store. We don't want to widen the integer operations only to fail to // promote due to some other unsplittable entry (which we may make splittable - // later). - bool WholeAllocaOp = false; + // later). However, if there are only splittable uses, go ahead and assume + // that we cover the alloca. + bool WholeAllocaOp = (I != E) ? false : TD.isLegalInteger(SizeInBits); for (; I != E; ++I) if (!isIntegerWideningViableForPartitioning(TD, AllocaTy, AllocBeginOffset, diff --git a/test/Transforms/SROA/basictest.ll b/test/Transforms/SROA/basictest.ll index ca52dceb8f0..9e5a26104db 100644 --- a/test/Transforms/SROA/basictest.ll +++ b/test/Transforms/SROA/basictest.ll @@ -1317,3 +1317,23 @@ define void @PR15805(i1 %a, i1 %b) { %cond = load i64* %cond.in, align 8 ret void } + +define void @PR16651(i8* %a) { +; This test case caused a crash due to the volatile memcpy in combination with +; lowering to integer loads and stores of a width other than that of the original +; memcpy. +; +; CHECK-LABEL: @PR16651( +; CHECK: alloca i16 +; CHECK: alloca i8 +; CHECK: alloca i8 +; CHECK: unreachable + +entry: + %b = alloca i32, align 4 + %b.cast = bitcast i32* %b to i8* + call void @llvm.memcpy.p0i8.p0i8.i32(i8* %b.cast, i8* %a, i32 4, i32 4, i1 true) + %b.gep = getelementptr inbounds i8* %b.cast, i32 2 + load i8* %b.gep, align 2 + unreachable +}