[llvm] r188119 - Kill some duplicated code for removing unreachable BBs.
Arnold Schwaighofer
aschwaighofer at apple.com
Sat Aug 10 12:52:23 PDT 2013
Hi Peter,
we run our tests with libgmalloc enabled on our internal buildbots. Following this commit (188119) the args-unreachable-bb.ll test fails:
Can you please fix this?
Thanks,
Arnold
Release+Asserts/bin/llvm-lit -v --param use_gmalloc=1 --param gmalloc_path=/usr/lib/libgmalloc.dylib ../test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll
Exit Code: 2
-- Testing: 1 tests, 1 threads --
FAIL: LLVM :: Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll (1 of 1)
******************** TEST 'LLVM :: Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll' FAILED ********************
Script:
--
/Volumes/Data/backedup/dev/os/buildczar/llvm-ToT/release/Release+Asserts/bin/opt < /Volumes/Data/backedup/dev/os/buildczar/llvm-ToT/test/Instrument
ation/DataFlowSanitizer/args-unreachable-bb.ll -dfsan -verify -dfsan-args-abi -S | /Volumes/Data/backedup/dev/os/buildczar/llvm-ToT/release/Release
+Asserts/bin/FileCheck /Volumes/Data/backedup/dev/os/buildczar/llvm-ToT/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll
--
Command Output (stderr):
--
GuardMalloc[bash-32348]: Allocations will be placed on 16 byte boundaries.
GuardMalloc[bash-32348]: - Some buffer overruns may not be noticed.
GuardMalloc[bash-32348]: - Applications using vector instructions (e.g., SSE) should work.
GuardMalloc[bash-32348]: version 27
GuardMalloc[FileCheck-32350]: Allocations will be placed on 16 byte boundaries.
GuardMalloc[FileCheck-32350]: - Some buffer overruns may not be noticed.
GuardMalloc[FileCheck-32350]: - Applications using vector instructions (e.g., SSE) should work.
GuardMalloc[FileCheck-32350]: version 27
GuardMalloc[opt-32349]: Allocations will be placed on 16 byte boundaries.
GuardMalloc[opt-32349]: - Some buffer overruns may not be noticed.
GuardMalloc[opt-32349]: - Applications using vector instructions (e.g., SSE) should work.
GuardMalloc[opt-32349]: version 27
0 opt 0x0000000103ffe198 llvm::sys::PrintStackTrace(__sFILE*) + 40
1 opt 0x0000000103ffe694 SignalHandler(int) + 548
2 libsystem_platform.dylib 0x00007fff902f25aa _sigtramp + 26
3 libsystem_platform.dylib 0xffffffffffffffff _sigtramp + 1875958383
4 opt 0x0000000103f99bb5 llvm::MPPassManager::runOnModule(llvm::Module&) + 517
5 opt 0x0000000103f9a24c llvm::PassManagerImpl::run(llvm::Module&) + 540
6 opt 0x0000000103f9a43d llvm::PassManager::run(llvm::Module&) + 13
7 opt 0x00000001033e9661 main + 7089
8 libdyld.dylib 0x00007fff92acb5fd start + 1
Stack dump:
0. Program arguments: /Volumes/Data/backedup/dev/os/buildczar/llvm-ToT/release/Release+Asserts/bin/opt -dfsan -verify -dfsan-args-abi -S
1. Running pass 'DataFlowSanitizer: dynamic data flow analysis.' on module '<stdin>'.
FileCheck error: '-' is empty.
--
********************
Testing Time: 0.07s
********************
Failing Tests (1):
LLVM :: Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll
Unexpected Failures: 1
On Aug 9, 2013, at 5:47 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> Author: pcc
> Date: Fri Aug 9 17:47:24 2013
> New Revision: 188119
>
> URL: http://llvm.org/viewvc/llvm-project?rev=188119&view=rev
> Log:
> Kill some duplicated code for removing unreachable BBs.
>
> This moves removeUnreachableBlocksFromFn from SimplifyCFGPass.cpp
> to Utils/Local.cpp and uses it to replace the implementation of
> llvm::removeUnreachableBlocks, which appears to do a strict subset
> of what removeUnreachableBlocksFromFn does.
>
> Differential Revision: http://llvm-reviews.chandlerc.com/D1334
>
> Modified:
> llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp
> llvm/trunk/lib/Transforms/Utils/Local.cpp
> llvm/trunk/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp?rev=188119&r1=188118&r2=188119&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/SimplifyCFGPass.cpp Fri Aug 9 17:47:24 2013
> @@ -66,161 +66,6 @@ FunctionPass *llvm::createCFGSimplificat
> return new CFGSimplifyPass();
> }
>
> -/// changeToUnreachable - Insert an unreachable instruction before the specified
> -/// instruction, making it and the rest of the code in the block dead.
> -static void changeToUnreachable(Instruction *I, bool UseLLVMTrap) {
> - BasicBlock *BB = I->getParent();
> - // Loop over all of the successors, removing BB's entry from any PHI
> - // nodes.
> - for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
> - (*SI)->removePredecessor(BB);
> -
> - // Insert a call to llvm.trap right before this. This turns the undefined
> - // behavior into a hard fail instead of falling through into random code.
> - if (UseLLVMTrap) {
> - Function *TrapFn =
> - Intrinsic::getDeclaration(BB->getParent()->getParent(), Intrinsic::trap);
> - CallInst *CallTrap = CallInst::Create(TrapFn, "", I);
> - CallTrap->setDebugLoc(I->getDebugLoc());
> - }
> - new UnreachableInst(I->getContext(), I);
> -
> - // All instructions after this are dead.
> - BasicBlock::iterator BBI = I, BBE = BB->end();
> - while (BBI != BBE) {
> - if (!BBI->use_empty())
> - BBI->replaceAllUsesWith(UndefValue::get(BBI->getType()));
> - BB->getInstList().erase(BBI++);
> - }
> -}
> -
> -/// changeToCall - Convert the specified invoke into a normal call.
> -static void changeToCall(InvokeInst *II) {
> - SmallVector<Value*, 8> Args(II->op_begin(), II->op_end() - 3);
> - CallInst *NewCall = CallInst::Create(II->getCalledValue(), Args, "", II);
> - NewCall->takeName(II);
> - NewCall->setCallingConv(II->getCallingConv());
> - NewCall->setAttributes(II->getAttributes());
> - NewCall->setDebugLoc(II->getDebugLoc());
> - II->replaceAllUsesWith(NewCall);
> -
> - // Follow the call by a branch to the normal destination.
> - BranchInst::Create(II->getNormalDest(), II);
> -
> - // Update PHI nodes in the unwind destination
> - II->getUnwindDest()->removePredecessor(II->getParent());
> - II->eraseFromParent();
> -}
> -
> -static bool markAliveBlocks(BasicBlock *BB,
> - SmallPtrSet<BasicBlock*, 128> &Reachable) {
> -
> - SmallVector<BasicBlock*, 128> Worklist;
> - Worklist.push_back(BB);
> - Reachable.insert(BB);
> - bool Changed = false;
> - do {
> - BB = Worklist.pop_back_val();
> -
> - // Do a quick scan of the basic block, turning any obviously unreachable
> - // instructions into LLVM unreachable insts. The instruction combining pass
> - // canonicalizes unreachable insts into stores to null or undef.
> - for (BasicBlock::iterator BBI = BB->begin(), E = BB->end(); BBI != E;++BBI){
> - if (CallInst *CI = dyn_cast<CallInst>(BBI)) {
> - if (CI->doesNotReturn()) {
> - // If we found a call to a no-return function, insert an unreachable
> - // instruction after it. Make sure there isn't *already* one there
> - // though.
> - ++BBI;
> - if (!isa<UnreachableInst>(BBI)) {
> - // Don't insert a call to llvm.trap right before the unreachable.
> - changeToUnreachable(BBI, false);
> - Changed = true;
> - }
> - break;
> - }
> - }
> -
> - // Store to undef and store to null are undefined and used to signal that
> - // they should be changed to unreachable by passes that can't modify the
> - // CFG.
> - if (StoreInst *SI = dyn_cast<StoreInst>(BBI)) {
> - // Don't touch volatile stores.
> - if (SI->isVolatile()) continue;
> -
> - Value *Ptr = SI->getOperand(1);
> -
> - if (isa<UndefValue>(Ptr) ||
> - (isa<ConstantPointerNull>(Ptr) &&
> - SI->getPointerAddressSpace() == 0)) {
> - changeToUnreachable(SI, true);
> - Changed = true;
> - break;
> - }
> - }
> - }
> -
> - // Turn invokes that call 'nounwind' functions into ordinary calls.
> - if (InvokeInst *II = dyn_cast<InvokeInst>(BB->getTerminator())) {
> - Value *Callee = II->getCalledValue();
> - if (isa<ConstantPointerNull>(Callee) || isa<UndefValue>(Callee)) {
> - changeToUnreachable(II, true);
> - Changed = true;
> - } else if (II->doesNotThrow()) {
> - if (II->use_empty() && II->onlyReadsMemory()) {
> - // jump to the normal destination branch.
> - BranchInst::Create(II->getNormalDest(), II);
> - II->getUnwindDest()->removePredecessor(II->getParent());
> - II->eraseFromParent();
> - } else
> - changeToCall(II);
> - Changed = true;
> - }
> - }
> -
> - Changed |= ConstantFoldTerminator(BB, true);
> - for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
> - if (Reachable.insert(*SI))
> - Worklist.push_back(*SI);
> - } while (!Worklist.empty());
> - return Changed;
> -}
> -
> -/// removeUnreachableBlocksFromFn - Remove blocks that are not reachable, even
> -/// if they are in a dead cycle. Return true if a change was made, false
> -/// otherwise.
> -static bool removeUnreachableBlocksFromFn(Function &F) {
> - SmallPtrSet<BasicBlock*, 128> Reachable;
> - bool Changed = markAliveBlocks(F.begin(), Reachable);
> -
> - // If there are unreachable blocks in the CFG...
> - if (Reachable.size() == F.size())
> - return Changed;
> -
> - assert(Reachable.size() < F.size());
> - NumSimpl += F.size()-Reachable.size();
> -
> - // Loop over all of the basic blocks that are not reachable, dropping all of
> - // their internal references...
> - for (Function::iterator BB = ++F.begin(), E = F.end(); BB != E; ++BB) {
> - if (Reachable.count(BB))
> - continue;
> -
> - for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
> - if (Reachable.count(*SI))
> - (*SI)->removePredecessor(BB);
> - BB->dropAllReferences();
> - }
> -
> - for (Function::iterator I = ++F.begin(); I != F.end();)
> - if (!Reachable.count(I))
> - I = F.getBasicBlockList().erase(I);
> - else
> - ++I;
> -
> - return true;
> -}
> -
> /// mergeEmptyReturnBlocks - If we have more than one empty (other than phi
> /// node) return blocks, merge them together to promote recursive block merging.
> static bool mergeEmptyReturnBlocks(Function &F) {
> @@ -325,7 +170,7 @@ static bool iterativelySimplifyCFG(Funct
> bool CFGSimplifyPass::runOnFunction(Function &F) {
> const TargetTransformInfo &TTI = getAnalysis<TargetTransformInfo>();
> const DataLayout *TD = getAnalysisIfAvailable<DataLayout>();
> - bool EverChanged = removeUnreachableBlocksFromFn(F);
> + bool EverChanged = removeUnreachableBlocks(F);
> EverChanged |= mergeEmptyReturnBlocks(F);
> EverChanged |= iterativelySimplifyCFG(F, TTI, TD);
>
> @@ -333,16 +178,16 @@ bool CFGSimplifyPass::runOnFunction(Func
> if (!EverChanged) return false;
>
> // iterativelySimplifyCFG can (rarely) make some loops dead. If this happens,
> - // removeUnreachableBlocksFromFn is needed to nuke them, which means we should
> + // removeUnreachableBlocks is needed to nuke them, which means we should
> // iterate between the two optimizations. We structure the code like this to
> // avoid reruning iterativelySimplifyCFG if the second pass of
> - // removeUnreachableBlocksFromFn doesn't do anything.
> - if (!removeUnreachableBlocksFromFn(F))
> + // removeUnreachableBlocks doesn't do anything.
> + if (!removeUnreachableBlocks(F))
> return true;
>
> do {
> EverChanged = iterativelySimplifyCFG(F, TTI, TD);
> - EverChanged |= removeUnreachableBlocksFromFn(F);
> + EverChanged |= removeUnreachableBlocks(F);
> } while (EverChanged);
>
> return true;
>
> Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=188119&r1=188118&r2=188119&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/Local.cpp Fri Aug 9 17:47:24 2013
> @@ -16,6 +16,7 @@
> #include "llvm/ADT/DenseMap.h"
> #include "llvm/ADT/STLExtras.h"
> #include "llvm/ADT/SmallPtrSet.h"
> +#include "llvm/ADT/Statistic.h"
> #include "llvm/Analysis/Dominators.h"
> #include "llvm/Analysis/InstructionSimplify.h"
> #include "llvm/Analysis/MemoryBuiltins.h"
> @@ -43,6 +44,8 @@
> #include "llvm/Support/raw_ostream.h"
> using namespace llvm;
>
> +STATISTIC(NumRemoved, "Number of unreachable basic blocks removed");
> +
> //===----------------------------------------------------------------------===//
> // Local constant propagation.
> //
> @@ -1121,33 +1124,153 @@ bool llvm::replaceDbgDeclareForAlloca(Al
> return true;
> }
>
> -bool llvm::removeUnreachableBlocks(Function &F) {
> - SmallPtrSet<BasicBlock*, 16> Reachable;
> +/// changeToUnreachable - Insert an unreachable instruction before the specified
> +/// instruction, making it and the rest of the code in the block dead.
> +static void changeToUnreachable(Instruction *I, bool UseLLVMTrap) {
> + BasicBlock *BB = I->getParent();
> + // Loop over all of the successors, removing BB's entry from any PHI
> + // nodes.
> + for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
> + (*SI)->removePredecessor(BB);
> +
> + // Insert a call to llvm.trap right before this. This turns the undefined
> + // behavior into a hard fail instead of falling through into random code.
> + if (UseLLVMTrap) {
> + Function *TrapFn =
> + Intrinsic::getDeclaration(BB->getParent()->getParent(), Intrinsic::trap);
> + CallInst *CallTrap = CallInst::Create(TrapFn, "", I);
> + CallTrap->setDebugLoc(I->getDebugLoc());
> + }
> + new UnreachableInst(I->getContext(), I);
> +
> + // All instructions after this are dead.
> + BasicBlock::iterator BBI = I, BBE = BB->end();
> + while (BBI != BBE) {
> + if (!BBI->use_empty())
> + BBI->replaceAllUsesWith(UndefValue::get(BBI->getType()));
> + BB->getInstList().erase(BBI++);
> + }
> +}
> +
> +/// changeToCall - Convert the specified invoke into a normal call.
> +static void changeToCall(InvokeInst *II) {
> + SmallVector<Value*, 8> Args(II->op_begin(), II->op_end() - 3);
> + CallInst *NewCall = CallInst::Create(II->getCalledValue(), Args, "", II);
> + NewCall->takeName(II);
> + NewCall->setCallingConv(II->getCallingConv());
> + NewCall->setAttributes(II->getAttributes());
> + NewCall->setDebugLoc(II->getDebugLoc());
> + II->replaceAllUsesWith(NewCall);
> +
> + // Follow the call by a branch to the normal destination.
> + BranchInst::Create(II->getNormalDest(), II);
> +
> + // Update PHI nodes in the unwind destination
> + II->getUnwindDest()->removePredecessor(II->getParent());
> + II->eraseFromParent();
> +}
> +
> +static bool markAliveBlocks(BasicBlock *BB,
> + SmallPtrSet<BasicBlock*, 128> &Reachable) {
> +
> SmallVector<BasicBlock*, 128> Worklist;
> - Worklist.push_back(&F.getEntryBlock());
> - Reachable.insert(&F.getEntryBlock());
> + Worklist.push_back(BB);
> + Reachable.insert(BB);
> + bool Changed = false;
> do {
> - BasicBlock *BB = Worklist.pop_back_val();
> + BB = Worklist.pop_back_val();
> +
> + // Do a quick scan of the basic block, turning any obviously unreachable
> + // instructions into LLVM unreachable insts. The instruction combining pass
> + // canonicalizes unreachable insts into stores to null or undef.
> + for (BasicBlock::iterator BBI = BB->begin(), E = BB->end(); BBI != E;++BBI){
> + if (CallInst *CI = dyn_cast<CallInst>(BBI)) {
> + if (CI->doesNotReturn()) {
> + // If we found a call to a no-return function, insert an unreachable
> + // instruction after it. Make sure there isn't *already* one there
> + // though.
> + ++BBI;
> + if (!isa<UnreachableInst>(BBI)) {
> + // Don't insert a call to llvm.trap right before the unreachable.
> + changeToUnreachable(BBI, false);
> + Changed = true;
> + }
> + break;
> + }
> + }
> +
> + // Store to undef and store to null are undefined and used to signal that
> + // they should be changed to unreachable by passes that can't modify the
> + // CFG.
> + if (StoreInst *SI = dyn_cast<StoreInst>(BBI)) {
> + // Don't touch volatile stores.
> + if (SI->isVolatile()) continue;
> +
> + Value *Ptr = SI->getOperand(1);
> +
> + if (isa<UndefValue>(Ptr) ||
> + (isa<ConstantPointerNull>(Ptr) &&
> + SI->getPointerAddressSpace() == 0)) {
> + changeToUnreachable(SI, true);
> + Changed = true;
> + break;
> + }
> + }
> + }
> +
> + // Turn invokes that call 'nounwind' functions into ordinary calls.
> + if (InvokeInst *II = dyn_cast<InvokeInst>(BB->getTerminator())) {
> + Value *Callee = II->getCalledValue();
> + if (isa<ConstantPointerNull>(Callee) || isa<UndefValue>(Callee)) {
> + changeToUnreachable(II, true);
> + Changed = true;
> + } else if (II->doesNotThrow()) {
> + if (II->use_empty() && II->onlyReadsMemory()) {
> + // jump to the normal destination branch.
> + BranchInst::Create(II->getNormalDest(), II);
> + II->getUnwindDest()->removePredecessor(II->getParent());
> + II->eraseFromParent();
> + } else
> + changeToCall(II);
> + Changed = true;
> + }
> + }
> +
> + Changed |= ConstantFoldTerminator(BB, true);
> for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
> if (Reachable.insert(*SI))
> Worklist.push_back(*SI);
> } while (!Worklist.empty());
> + return Changed;
> +}
> +
> +/// removeUnreachableBlocksFromFn - Remove blocks that are not reachable, even
> +/// if they are in a dead cycle. Return true if a change was made, false
> +/// otherwise.
> +bool llvm::removeUnreachableBlocks(Function &F) {
> + SmallPtrSet<BasicBlock*, 128> Reachable;
> + bool Changed = markAliveBlocks(F.begin(), Reachable);
>
> + // If there are unreachable blocks in the CFG...
> if (Reachable.size() == F.size())
> - return false;
> + return Changed;
>
> assert(Reachable.size() < F.size());
> - for (Function::iterator I = llvm::next(F.begin()), E = F.end(); I != E; ++I) {
> - if (Reachable.count(I))
> + NumRemoved += F.size()-Reachable.size();
> +
> + // Loop over all of the basic blocks that are not reachable, dropping all of
> + // their internal references...
> + for (Function::iterator BB = ++F.begin(), E = F.end(); BB != E; ++BB) {
> + if (Reachable.count(BB))
> continue;
>
> - for (succ_iterator SI = succ_begin(I), SE = succ_end(I); SI != SE; ++SI)
> + for (succ_iterator SI = succ_begin(BB), SE = succ_end(BB); SI != SE; ++SI)
> if (Reachable.count(*SI))
> - (*SI)->removePredecessor(I);
> - I->dropAllReferences();
> + (*SI)->removePredecessor(BB);
> + BB->dropAllReferences();
> }
>
> - for (Function::iterator I = llvm::next(F.begin()), E=F.end(); I != E;)
> + for (Function::iterator I = ++F.begin(); I != F.end();)
> if (!Reachable.count(I))
> I = F.getBasicBlockList().erase(I);
> else
>
> Modified: llvm/trunk/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll?rev=188119&r1=188118&r2=188119&view=diff
> ==============================================================================
> --- llvm/trunk/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll (original)
> +++ llvm/trunk/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll Fri Aug 9 17:47:24 2013
> @@ -1,8 +1,8 @@
> ; RUN: opt < %s -dfsan -verify -dfsan-args-abi -S | FileCheck %s
> 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"
>
> -define i8 @unreachable_bb() {
> - ; CHECK: @unreachable_bb
> +; CHECK-LABEL: @unreachable_bb1
> +define i8 @unreachable_bb1() {
> ; CHECK: ret { i8, i16 } { i8 1, i16 0 }
> ; CHECK-NOT: bb2:
> ; CHECK-NOT: bb3:
> @@ -18,3 +18,13 @@ bb3:
> bb4:
> br label %bb3
> }
> +
> +declare void @abort() noreturn
> +
> +; CHECK-LABEL: @unreachable_bb2
> +define i8 @unreachable_bb2() {
> + call void @abort() noreturn
> + ; CHECK-NOT: i8 12
> + ; CHECK: unreachable
> + ret i8 12
> +}
>
>
> _______________________________________________
> 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