[llvm-commits] [llvm] r154157 - in /llvm/trunk: lib/Transforms/Utils/CloneFunction.cpp test/Transforms/Inline/inline_cleanup.ll

Daniel Dunbar daniel at zuster.org
Thu Apr 5 21:58:50 PDT 2012


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




More information about the llvm-commits mailing list