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