From 0fa125a77d4c960837c36c30ce8cecf8262f35e9 Mon Sep 17 00:00:00 2001 From: Jingyue Wu Date: Sun, 16 Nov 2014 16:52:44 +0000 Subject: [PATCH] [DependenceAnalysis] Allow subscripts of different types Summary: Several places in DependenceAnalysis assumes both SCEVs in a subscript pair share the same integer type. For instance, isKnownPredicate calls SE->getMinusSCEV(X, Y) which asserts X and Y share the same type. However, DependenceAnalysis fails to ensure this assumption when producing a subscript pair, causing tests such as NonCanonicalizedSubscript to crash. With this patch, DependenceAnalysis runs unifySubscriptType before producing any subscript pair, ensuring the assumption. Test Plan: Added NonCanonicalizedSubscript.ll on which DependenceAnalysis before the fix crashed because subscripts have different types. Reviewers: spop, sebpop, jingyue Reviewed By: jingyue Subscribers: eliben, meheff, llvm-commits Differential Revision: http://reviews.llvm.org/D6289 llvm-svn: 222100 --- .../llvm/Analysis/DependenceAnalysis.h | 8 +++- llvm/lib/Analysis/DependenceAnalysis.cpp | 31 ++++++++++++-- .../NonCanonicalizedSubscript.ll | 40 +++++++++++++++++++ 3 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 llvm/test/Analysis/DependenceAnalysis/NonCanonicalizedSubscript.ll diff --git a/llvm/include/llvm/Analysis/DependenceAnalysis.h b/llvm/include/llvm/Analysis/DependenceAnalysis.h index a52d6a8ef2d8..1041e3f0a4a9 100644 --- a/llvm/include/llvm/Analysis/DependenceAnalysis.h +++ b/llvm/include/llvm/Analysis/DependenceAnalysis.h @@ -523,6 +523,12 @@ namespace llvm { /// in LoopNest. bool isLoopInvariant(const SCEV *Expression, const Loop *LoopNest) const; + /// Makes sure both subscripts (i.e. Pair->Src and Pair->Dst) share the same + /// integer type by sign-extending one of them when necessary. + /// Sign-extending a subscript is safe because getelementptr assumes the + /// array subscripts are signed. + void unifySubscriptType(Subscript *Pair); + /// removeMatchingExtensions - Examines a subscript pair. /// If the source and destination are identically sign (or zero) /// extended, it strips off the extension in an effort to @@ -911,7 +917,7 @@ namespace llvm { bool tryDelinearize(const SCEV *SrcSCEV, const SCEV *DstSCEV, SmallVectorImpl &Pair, - const SCEV *ElementSize) const; + const SCEV *ElementSize); public: static char ID; // Class identification, replacement for typeinfo diff --git a/llvm/lib/Analysis/DependenceAnalysis.cpp b/llvm/lib/Analysis/DependenceAnalysis.cpp index 5ac3f38c0c54..092df5c15cde 100644 --- a/llvm/lib/Analysis/DependenceAnalysis.cpp +++ b/llvm/lib/Analysis/DependenceAnalysis.cpp @@ -781,6 +781,25 @@ void DependenceAnalysis::collectCommonLoops(const SCEV *Expression, } } +void DependenceAnalysis::unifySubscriptType(Subscript *Pair) { + const SCEV *Src = Pair->Src; + const SCEV *Dst = Pair->Dst; + IntegerType *SrcTy = dyn_cast(Src->getType()); + IntegerType *DstTy = dyn_cast(Dst->getType()); + if (SrcTy == nullptr || DstTy == nullptr) { + assert(SrcTy == DstTy && "This function only unify integer types and " + "expect Src and Dst share the same type " + "otherwise."); + return; + } + if (SrcTy->getBitWidth() > DstTy->getBitWidth()) { + // Sign-extend Dst to typeof(Src) if typeof(Src) is wider than typeof(Dst). + Pair->Dst = SE->getSignExtendExpr(Dst, SrcTy); + } else if (SrcTy->getBitWidth() < DstTy->getBitWidth()) { + // Sign-extend Src to typeof(Dst) if typeof(Dst) is wider than typeof(Src). + Pair->Src = SE->getSignExtendExpr(Src, DstTy); + } +} // removeMatchingExtensions - Examines a subscript pair. // If the source and destination are identically sign (or zero) @@ -793,9 +812,11 @@ void DependenceAnalysis::removeMatchingExtensions(Subscript *Pair) { (isa(Src) && isa(Dst))) { const SCEVCastExpr *SrcCast = cast(Src); const SCEVCastExpr *DstCast = cast(Dst); - if (SrcCast->getType() == DstCast->getType()) { - Pair->Src = SrcCast->getOperand(); - Pair->Dst = DstCast->getOperand(); + const SCEV *SrcCastOp = SrcCast->getOperand(); + const SCEV *DstCastOp = DstCast->getOperand(); + if (SrcCastOp->getType() == DstCastOp->getType()) { + Pair->Src = SrcCastOp; + Pair->Dst = DstCastOp; } } } @@ -3178,7 +3199,7 @@ void DependenceAnalysis::updateDirection(Dependence::DVEntry &Level, bool DependenceAnalysis::tryDelinearize(const SCEV *SrcSCEV, const SCEV *DstSCEV, SmallVectorImpl &Pair, - const SCEV *ElementSize) const { + const SCEV *ElementSize) { const SCEVUnknown *SrcBase = dyn_cast(SE->getPointerBase(SrcSCEV)); const SCEVUnknown *DstBase = @@ -3233,6 +3254,7 @@ bool DependenceAnalysis::tryDelinearize(const SCEV *SrcSCEV, for (int i = 0; i < size; ++i) { Pair[i].Src = SrcSubscripts[i]; Pair[i].Dst = DstSubscripts[i]; + unifySubscriptType(&Pair[i]); // FIXME: we should record the bounds SrcSizes[i] and DstSizes[i] that the // delinearization has found, and add these constraints to the dependence @@ -3341,6 +3363,7 @@ DependenceAnalysis::depends(Instruction *Src, Instruction *Dst, ++SrcIdx, ++DstIdx, ++P) { Pair[P].Src = SE->getSCEV(*SrcIdx); Pair[P].Dst = SE->getSCEV(*DstIdx); + unifySubscriptType(&Pair[P]); } } else { diff --git a/llvm/test/Analysis/DependenceAnalysis/NonCanonicalizedSubscript.ll b/llvm/test/Analysis/DependenceAnalysis/NonCanonicalizedSubscript.ll new file mode 100644 index 000000000000..95e5e52bc2a7 --- /dev/null +++ b/llvm/test/Analysis/DependenceAnalysis/NonCanonicalizedSubscript.ll @@ -0,0 +1,40 @@ +; RUN: opt < %s -analyze -basicaa -da -da-delinearize=false | FileCheck %s +; RUN: opt < %s -analyze -basicaa -da -da-delinearize | FileCheck %s -check-prefix=DELIN + +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" +target triple = "x86_64-apple-macosx10.6.0" + +; for (int i = 0; i < 100; ++i) { +; int t0 = a[i][i]; +; int t1 = t0 + 1; +; a[i][5] = t1; +; } +; The subscript 5 in a[i][5] is deliberately an i32, mismatching the types of +; other subscript. DependenceAnalysis before the fix crashed due to this +; mismatch. +define void @i32_subscript([100 x [100 x i32]]* %a, i32* %b) { +; CHECK-LABEL: 'Dependence Analysis' for function 'i32_subscript' +; DELIN-LABEL: 'Dependence Analysis' for function 'i32_subscript' +entry: + br label %for.body + +for.body: +; CHECK: da analyze - none! +; CHECK: da analyze - anti [=|<]! +; CHECK: da analyze - none! +; DELIN: da analyze - none! +; DELIN: da analyze - anti [=|<]! +; DELIN: da analyze - none! + %i = phi i64 [ 0, %entry ], [ %i.inc, %for.body ] + %a.addr = getelementptr [100 x [100 x i32]]* %a, i64 0, i64 %i, i64 %i + %a.addr.2 = getelementptr [100 x [100 x i32]]* %a, i64 0, i64 %i, i32 5 + %0 = load i32* %a.addr, align 4 + %1 = add i32 %0, 1 + store i32 %1, i32* %a.addr.2, align 4 + %i.inc = add nsw i64 %i, 1 + %exitcond = icmp ne i64 %i.inc, 100 + br i1 %exitcond, label %for.body, label %for.end + +for.end: + ret void +}