From: Chandler Carruth Date: Tue, 25 Feb 2014 21:24:45 +0000 (+0000) Subject: [SROA] Add an off-by-default *strict* inbounds check to SROA. I had SROA X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=495e40121b5d0d460545c92ccb492d8b5e005cb3;p=oota-llvm.git [SROA] Add an off-by-default *strict* inbounds check to SROA. I had SROA implemented this way a long time ago and due to the overwhelming bugs that surfaced, moved to a much more relaxed variant. Richard Smith would like to understand the magnitude of this problem and it seems fairly harmless to keep some flag-controlled logic to get the extremely strict behavior here. I'll remove it if it doesn't prove useful. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@202193 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Transforms/Scalar/SROA.cpp b/lib/Transforms/Scalar/SROA.cpp index 49d644a4424..01320e22a3d 100644 --- a/lib/Transforms/Scalar/SROA.cpp +++ b/lib/Transforms/Scalar/SROA.cpp @@ -85,6 +85,11 @@ ForceSSAUpdater("force-ssa-updater", cl::init(false), cl::Hidden); static cl::opt SROARandomShuffleSlices("sroa-random-shuffle-slices", cl::init(false), cl::Hidden); +/// Hidden option to experiment with completely strict handling of inbounds +/// GEPs. +static cl::opt SROAStrictInbounds("sroa-strict-inbounds", + cl::init(false), cl::Hidden); + namespace { /// \brief A custom IRBuilder inserter which prefixes all names if they are /// preserved. @@ -392,6 +397,43 @@ private: if (GEPI.use_empty()) return markAsDead(GEPI); + if (SROAStrictInbounds && GEPI.isInBounds()) { + // FIXME: This is a manually un-factored variant of the basic code inside + // of GEPs with checking of the inbounds invariant specified in the + // langref in a very strict sense. If we ever want to enable + // SROAStrictInbounds, this code should be factored cleanly into + // PtrUseVisitor, but it is easier to experiment with SROAStrictInbounds + // by writing out the code here where we have tho underlying allocation + // size readily available. + APInt GEPOffset = Offset; + for (gep_type_iterator GTI = gep_type_begin(GEPI), + GTE = gep_type_end(GEPI); + GTI != GTE; ++GTI) { + ConstantInt *OpC = dyn_cast(GTI.getOperand()); + if (!OpC) + break; + + // Handle a struct index, which adds its field offset to the pointer. + if (StructType *STy = dyn_cast(*GTI)) { + unsigned ElementIdx = OpC->getZExtValue(); + const StructLayout *SL = DL.getStructLayout(STy); + GEPOffset += + APInt(Offset.getBitWidth(), SL->getElementOffset(ElementIdx)); + } else { + // For array or vector indices, scale the index by the size of the type. + APInt Index = OpC->getValue().sextOrTrunc(Offset.getBitWidth()); + GEPOffset += Index * APInt(Offset.getBitWidth(), + DL.getTypeAllocSize(GTI.getIndexedType())); + } + + // If this index has computed an intermediate pointer which is not + // inbounds, then the result of the GEP is a poison value and we can + // delete it and all uses. + if (GEPOffset.ugt(AllocSize)) + return markAsDead(GEPI); + } + } + return Base::visitGetElementPtrInst(GEPI); }