[llvm-commits] [llvm] r154157 - in /llvm/trunk: lib/Transforms/Utils/CloneFunction.cpp test/Transforms/Inline/inline_cleanup.ll
Chandler Carruth
chandlerc at gmail.com
Fri Apr 6 10:24:23 PDT 2012
Fixed in r154179, thanks for the great test case!
On Fri, Apr 6, 2012 at 6:58 AM, Daniel Dunbar <daniel at zuster.org> wrote:
> Hey Chandler,
>
> This is causing the compiler to crash on one of our test cases, here
> is a reduced version:
> --
> $ cat t.c
> void f0(unsigned char a[4][8], unsigned char d, unsigned char e) {
> unsigned char tmp[8];
> int i, j;
>
> for(i = 1; i < 4; i++) {
> for(j = 0; j < e; j++) tmp[j] = a[i][(j + 0) % e];
> for(j = 0; j < e; j++) a[i][j] = tmp[j];
> }
> }
>
> void f1(unsigned char a[4][8]) {
> for (unsigned r = 1; r != 8; r++)
> f0(a,0,8);
> }
> $ daclang -O3 -c t.c
> Assertion failed: (InsertBefore->getParent() && "Instruction to insert
> before is not in a basic block!"), function Instruction, file
> /Users/ddunbar/llvm/lib/VMCore/Instruction.cpp, line 32.
> 0 clang 0x0000000102208efe _ZL15PrintStackTracePv + 46
> 1 clang 0x00000001022094a9 _ZL13SignalHandleri + 297
> 2 libsystem_c.dylib 0x00007fff95c99cfa _sigtramp + 26
> 3 libsystem_c.dylib 0x0000040000000018 _sigtramp + 18446607736049656632
> 4 clang 0x00000001022091cb raise + 27
> 5 clang 0x0000000102209282 abort + 18
> 6 clang 0x0000000102209261 __assert_rtn + 129
> 7 clang 0x0000000100135df0
> llvm::Instruction::Instruction(llvm::Type*, unsigned int, llvm::Use*,
> unsigned int, llvm::Instruction*) + 256
> 8 clang 0x000000010165ff13
> llvm::TerminatorInst::TerminatorInst(llvm::Type*,
> llvm::Instruction::TermOps, llvm::Use*, unsigned int,
> llvm::Instruction*) + 83
> 9 clang 0x000000010015c206
> llvm::BranchInst::BranchInst(llvm::BasicBlock*, llvm::Instruction*) +
> 118
> 10 clang 0x000000010015c185
> llvm::BranchInst::BranchInst(llvm::BasicBlock*, llvm::Instruction*) +
> 37
> 11 clang 0x0000000101efe10d
> llvm::BranchInst::Create(llvm::BasicBlock*, llvm::Instruction*) + 61
> 12 clang 0x00000001003dcfd8
> llvm::InlineFunction(llvm::CallSite, llvm::InlineFunctionInfo&, bool)
> + 5752
> 13 clang 0x000000010169bdd3
>
> _ZL20InlineCallIfPossibleN4llvm8CallSiteERNS_18InlineFunctionInfoERNS_8DenseMapIPNS_9ArrayTypeESt6vectorIPNS_10AllocaInstESaIS8_EENS_12DenseMapInfoIS5_EENSB_ISA_EEEEib
> + 99
> --
>
> I looked into a bit to see if I could fix it easily, but I don't
> understand the code enough. What is ultimately happening is that we
> CloneAndPruneFunctionInto is returning with two elements in the
> Returns array, but they are identical. I believe what happens is that
> we add the first return, then later we fold the instructions from that
> BB into another BB and add it to the Returns array again. I'll let you
> figure out the right fix. :)
>
> - Daniel
>
> On Thu, Apr 5, 2012 at 6:11 PM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
> > Author: chandlerc
> > Date: Thu Apr 5 20:11:52 2012
> > New Revision: 154157
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=154157&view=rev
> > Log:
> > Sink the return instruction collection until after we're done deleting
> > dead code, including dead return instructions in some cases. Otherwise,
> > we end up having a bogus poniter to a return instruction that blows up
> > much further down the road.
> >
> > It turns out that this pattern is both simpler to code, easier to update
> > in the face of enhancements to the inliner cleanup, and likely cheaper
> > given that it won't add dead instructions to the list.
> >
> > Thanks to John Regehr's numerous test cases for teasing this out.
> >
> > Modified:
> > llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
> > llvm/trunk/test/Transforms/Inline/inline_cleanup.ll
> >
> > Modified: llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp?rev=154157&r1=154156&r2=154157&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Utils/CloneFunction.cpp Thu Apr 5
> 20:11:52 2012
> > @@ -200,7 +200,6 @@
> > const Function *OldFunc;
> > ValueToValueMapTy &VMap;
> > bool ModuleLevelChanges;
> > - SmallVectorImpl<ReturnInst*> &Returns;
> > const char *NameSuffix;
> > ClonedCodeInfo *CodeInfo;
> > const TargetData *TD;
> > @@ -208,13 +207,12 @@
> > PruningFunctionCloner(Function *newFunc, const Function *oldFunc,
> > ValueToValueMapTy &valueMap,
> > bool moduleLevelChanges,
> > - SmallVectorImpl<ReturnInst*> &returns,
> > const char *nameSuffix,
> > ClonedCodeInfo *codeInfo,
> > const TargetData *td)
> > : NewFunc(newFunc), OldFunc(oldFunc),
> > VMap(valueMap), ModuleLevelChanges(moduleLevelChanges),
> > - Returns(returns), NameSuffix(nameSuffix), CodeInfo(codeInfo),
> TD(td) {
> > + NameSuffix(nameSuffix), CodeInfo(codeInfo), TD(td) {
> > }
> >
> > /// CloneBlock - The specified block is found to be reachable, clone
> it and
> > @@ -352,9 +350,6 @@
> > CodeInfo->ContainsDynamicAllocas |= hasStaticAllocas &&
> > BB != &BB->getParent()->front();
> > }
> > -
> > - if (ReturnInst *RI = dyn_cast<ReturnInst>(NewBB->getTerminator()))
> > - Returns.push_back(RI);
> > }
> >
> > /// CloneAndPruneFunctionInto - This works exactly like
> CloneFunctionInto,
> > @@ -381,7 +376,7 @@
> > #endif
> >
> > PruningFunctionCloner PFC(NewFunc, OldFunc, VMap, ModuleLevelChanges,
> > - Returns, NameSuffix, CodeInfo, TD);
> > + NameSuffix, CodeInfo, TD);
> >
> > // Clone the entry block, and anything recursively reachable from it.
> > std::vector<const BasicBlock*> CloneWorklist;
> > @@ -537,6 +532,13 @@
> > // and we still want to prune the dead code as early as possible.
> > ConstantFoldTerminator(I);
> >
> > + // Track all of the newly-inserted returns.
> > + if (ReturnInst *RI = dyn_cast<ReturnInst>(I->getTerminator())) {
> > + Returns.push_back(RI);
> > + ++I;
> > + continue;
> > + }
> > +
> > BranchInst *BI = dyn_cast<BranchInst>(I->getTerminator());
> > if (!BI || BI->isConditional()) { ++I; continue; }
> >
> >
> > Modified: llvm/trunk/test/Transforms/Inline/inline_cleanup.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline_cleanup.ll?rev=154157&r1=154156&r2=154157&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/Transforms/Inline/inline_cleanup.ll (original)
> > +++ llvm/trunk/test/Transforms/Inline/inline_cleanup.ll Thu Apr 5
> 20:11:52 2012
> > @@ -136,3 +136,40 @@
> > call void @inner2(i32 0, i32 -1, i32 %z, i1 %b)
> > ret void
> > }
> > +
> > +define void @PR12470_inner(i16 signext %p1) nounwind uwtable {
> > +entry:
> > + br i1 undef, label %cond.true, label %cond.false
> > +
> > +cond.true:
> > + br label %cond.end
> > +
> > +cond.false:
> > + %conv = sext i16 %p1 to i32
> > + br label %cond.end
> > +
> > +cond.end:
> > + %cond = phi i32 [ undef, %cond.true ], [ 0, %cond.false ]
> > + %tobool = icmp eq i32 %cond, 0
> > + br i1 %tobool, label %if.end5, label %if.then
> > +
> > +if.then:
> > + ret void
> > +
> > +if.end5:
> > + ret void
> > +}
> > +
> > +define void @PR12470_outer() {
> > +; This previously crashed during inliner cleanup and folding inner
> return
> > +; instructions. Check that we don't crash and we produce a function
> with a single
> > +; crash.
> > +; CHECK: define void @PR12470_outer
> > +; CHECK: ret void
> > +; CHECK-NOT: ret void
> > +; CHECK: }
> > +
> > +entry:
> > + call void @PR12470_inner(i16 signext 1)
> > + ret void
> > +}
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120406/8fd0123c/attachment.html>
More information about the llvm-commits
mailing list