Skip to content

Commit

Permalink
Do not select EhPad BB in MachineBlockPlacement when there is regular…
Browse files Browse the repository at this point in the history 
… BB to schedule

Summary:
EHPad BB are not entered the classic way and therefor do not need to be placed after their predecessors. This patch make sure EHPad BB are not chosen amongst successors to form chains, and are selected as last resort when selecting the best candidate.

EHPad are scheduled in reverse probability order in order to have them flow into each others naturally.

Reviewers: chandlerc, majnemer, rafael, MatzeB, escha, silvas

Subscribers: llvm-commits

Differential Revision: http://reviews.llvm.org/D17625

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@265726 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
@deadalnix
deadalnix committed Apr 7, 2016
1 parent a8e98eb commit a5bbcb5
Show file tree
Hide file tree
Showing 6 changed files with 170 additions and 75 deletions.
78 changes: 66 additions & 12 deletions lib/CodeGen/MachineBlockPlacement.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ class MachineBlockPlacement : public MachineFunctionPass {

void markChainSuccessors(BlockChain &Chain, MachineBasicBlock *LoopHeaderBB,
SmallVectorImpl<MachineBasicBlock *> &BlockWorkList,
SmallVectorImpl<MachineBasicBlock *> &EHPadWorkList,
const BlockFilterSet *BlockFilter = nullptr);
MachineBasicBlock *selectBestSuccessor(MachineBasicBlock *BB,
BlockChain &Chain,
Expand All @@ -282,9 +283,11 @@ class MachineBlockPlacement : public MachineFunctionPass {
void fillWorkLists(MachineBasicBlock *MBB,
SmallPtrSetImpl<BlockChain *> &UpdatedPreds,
SmallVectorImpl<MachineBasicBlock *> &BlockWorkList,
SmallVectorImpl<MachineBasicBlock *> &EHPadWorkList,
const BlockFilterSet *BlockFilter);
void buildChain(MachineBasicBlock *BB, BlockChain &Chain,
SmallVectorImpl<MachineBasicBlock *> &BlockWorkList,
SmallVectorImpl<MachineBasicBlock *> &EHPadWorkList,
const BlockFilterSet *BlockFilter = nullptr);
MachineBasicBlock *findBestLoopTop(MachineLoop &L,
const BlockFilterSet &LoopBlockSet);
Expand Down Expand Up @@ -350,6 +353,7 @@ static std::string getBlockName(MachineBasicBlock *BB) {
void MachineBlockPlacement::markChainSuccessors(
BlockChain &Chain, MachineBasicBlock *LoopHeaderBB,
SmallVectorImpl<MachineBasicBlock *> &BlockWorkList,
SmallVectorImpl<MachineBasicBlock *> &EHPadWorkList,
const BlockFilterSet *BlockFilter) {
// Walk all the blocks in this chain, marking their successors as having
// a predecessor placed.
Expand All @@ -368,8 +372,15 @@ void MachineBlockPlacement::markChainSuccessors(

// This is a cross-chain edge that is within the loop, so decrement the
// loop predecessor count of the destination chain.
if (SuccChain.UnscheduledPredecessors > 0 && --SuccChain.UnscheduledPredecessors == 0)
BlockWorkList.push_back(*SuccChain.begin());
if (SuccChain.UnscheduledPredecessors == 0 ||
--SuccChain.UnscheduledPredecessors > 0)
continue;

auto *MBB = *SuccChain.begin();
if (MBB->isEHPad())
EHPadWorkList.push_back(MBB);
else
BlockWorkList.push_back(MBB);
}
}
}
Expand Down Expand Up @@ -412,7 +423,7 @@ MachineBlockPlacement::selectBestSuccessor(MachineBasicBlock *BB,
SmallVector<MachineBasicBlock *, 4> Successors;
for (MachineBasicBlock *Succ : BB->successors()) {
bool SkipSucc = false;
if (BlockFilter && !BlockFilter->count(Succ)) {
if (Succ->isEHPad() || (BlockFilter && !BlockFilter->count(Succ))) {
SkipSucc = true;
} else {
BlockChain *SuccChain = BlockToChain[Succ];
Expand Down Expand Up @@ -532,9 +543,16 @@ MachineBasicBlock *MachineBlockPlacement::selectBestCandidateBlock(
}),
WorkList.end());

if (WorkList.empty())
return nullptr;

bool IsEHPad = WorkList[0]->isEHPad();

MachineBasicBlock *BestBlock = nullptr;
BlockFrequency BestFreq;
for (MachineBasicBlock *MBB : WorkList) {
assert(MBB->isEHPad() == IsEHPad);

BlockChain &SuccChain = *BlockToChain[MBB];
if (&SuccChain == &Chain)
continue;
Expand All @@ -544,11 +562,32 @@ MachineBasicBlock *MachineBlockPlacement::selectBestCandidateBlock(
BlockFrequency CandidateFreq = MBFI->getBlockFreq(MBB);
DEBUG(dbgs() << " " << getBlockName(MBB) << " -> ";
MBFI->printBlockFreq(dbgs(), CandidateFreq) << " (freq)\n");
if (BestBlock && BestFreq >= CandidateFreq)

// For ehpad, we layout the least probable first as to avoid jumping back
// from least probable landingpads to more probable ones.
//
// FIXME: Using probability is probably (!) not the best way to achieve
// this. We should probably have a more principled approach to layout
// cleanup code.
//
// The goal is to get:
//
// +--------------------------+
// | V
// InnerLp -> InnerCleanup OuterLp -> OuterCleanup -> Resume
//
// Rather than:
//
// +-------------------------------------+
// V |
// OuterLp -> OuterCleanup -> Resume InnerLp -> InnerCleanup
if (BestBlock && (IsEHPad ^ (BestFreq >= CandidateFreq)))
continue;

BestBlock = MBB;
BestFreq = CandidateFreq;
}

return BestBlock;
}

Expand Down Expand Up @@ -582,6 +621,7 @@ void MachineBlockPlacement::fillWorkLists(
MachineBasicBlock *MBB,
SmallPtrSetImpl<BlockChain *> &UpdatedPreds,
SmallVectorImpl<MachineBasicBlock *> &BlockWorkList,
SmallVectorImpl<MachineBasicBlock *> &EHPadWorkList,
const BlockFilterSet *BlockFilter = nullptr) {
BlockChain &Chain = *BlockToChain[MBB];
if (!UpdatedPreds.insert(&Chain).second)
Expand All @@ -599,21 +639,29 @@ void MachineBlockPlacement::fillWorkLists(
}
}

if (Chain.UnscheduledPredecessors == 0)
BlockWorkList.push_back(*Chain.begin());
if (Chain.UnscheduledPredecessors != 0)
return;

MBB = *Chain.begin();
if (MBB->isEHPad())
EHPadWorkList.push_back(MBB);
else
BlockWorkList.push_back(MBB);
}

void MachineBlockPlacement::buildChain(
MachineBasicBlock *BB, BlockChain &Chain,
SmallVectorImpl<MachineBasicBlock *> &BlockWorkList,
SmallVectorImpl<MachineBasicBlock *> &EHPadWorkList,
const BlockFilterSet *BlockFilter) {
assert(BB);
assert(BlockToChain[BB] == &Chain);
MachineFunction &F = *BB->getParent();
MachineFunction::iterator PrevUnplacedBlockIt = F.begin();

MachineBasicBlock *LoopHeaderBB = BB;
markChainSuccessors(Chain, LoopHeaderBB, BlockWorkList, BlockFilter);
markChainSuccessors(Chain, LoopHeaderBB, BlockWorkList, EHPadWorkList,
BlockFilter);
BB = *std::prev(Chain.end());
for (;;) {
assert(BB);
Expand All @@ -629,6 +677,8 @@ void MachineBlockPlacement::buildChain(
// this point. This won't be a fallthrough, but it will increase locality.
if (!BestSucc)
BestSucc = selectBestCandidateBlock(Chain, BlockWorkList);
if (!BestSucc)
BestSucc = selectBestCandidateBlock(Chain, EHPadWorkList);

if (!BestSucc) {
BestSucc =
Expand All @@ -647,7 +697,8 @@ void MachineBlockPlacement::buildChain(
SuccChain.UnscheduledPredecessors = 0;
DEBUG(dbgs() << "Merging from " << getBlockName(BB) << " to "
<< getBlockName(BestSucc) << "\n");
markChainSuccessors(SuccChain, LoopHeaderBB, BlockWorkList, BlockFilter);
markChainSuccessors(SuccChain, LoopHeaderBB, BlockWorkList, EHPadWorkList,
BlockFilter);
Chain.merge(BestSucc, &SuccChain);
BB = *std::prev(Chain.end());
}
Expand Down Expand Up @@ -1067,6 +1118,7 @@ void MachineBlockPlacement::buildLoopChains(MachineFunction &F,
buildLoopChains(F, *InnerLoop);

SmallVector<MachineBasicBlock *, 16> BlockWorkList;
SmallVector<MachineBasicBlock *, 16> EHPadWorkList;
BlockFilterSet LoopBlockSet = collectLoopBlockSet(F, L);

// Check if we have profile data for this function. If yes, we will rotate
Expand Down Expand Up @@ -1100,9 +1152,10 @@ void MachineBlockPlacement::buildLoopChains(MachineFunction &F,
UpdatedPreds.insert(&LoopChain);

for (MachineBasicBlock *LoopBB : LoopBlockSet)
fillWorkLists(LoopBB, UpdatedPreds, BlockWorkList, &LoopBlockSet);
fillWorkLists(LoopBB, UpdatedPreds, BlockWorkList, EHPadWorkList,
&LoopBlockSet);

buildChain(LoopTop, LoopChain, BlockWorkList, &LoopBlockSet);
buildChain(LoopTop, LoopChain, BlockWorkList, EHPadWorkList, &LoopBlockSet);

if (RotateLoopWithProfile)
rotateLoopWithProfile(LoopChain, L, LoopBlockSet);
Expand Down Expand Up @@ -1199,13 +1252,14 @@ void MachineBlockPlacement::buildCFGChains(MachineFunction &F) {
buildLoopChains(F, *L);

SmallVector<MachineBasicBlock *, 16> BlockWorkList;
SmallVector<MachineBasicBlock *, 16> EHPadWorkList;

SmallPtrSet<BlockChain *, 4> UpdatedPreds;
for (MachineBasicBlock &MBB : F)
fillWorkLists(&MBB, UpdatedPreds, BlockWorkList);
fillWorkLists(&MBB, UpdatedPreds, BlockWorkList, EHPadWorkList);

BlockChain &FunctionChain = *BlockToChain[&F.front()];
buildChain(&F.front(), FunctionChain, BlockWorkList);
buildChain(&F.front(), FunctionChain, BlockWorkList, EHPadWorkList);

#ifndef NDEBUG
typedef SmallPtrSet<MachineBasicBlock *, 16> FunctionBlockSetType;
Expand Down
52 changes: 0 additions & 52 deletions test/CodeGen/PowerPC/pr25802.ll
View file

This file was deleted.

93 changes: 93 additions & 0 deletions test/CodeGen/X86/block-placement.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1083,3 +1083,96 @@ exit:
%ret = phi i32 [ %val1, %then ], [ %val2, %else ]
ret i32 %ret
}

; Make sure we put landingpads out of the way.
declare i32 @pers(...)

declare i32 @foo();

declare i32 @bar();

define i32 @test_lp(i32 %a) personality i32 (...)* @pers {
; CHECK-LABEL: test_lp:
; CHECK: %entry
; CHECK: %hot
; CHECK: %then
; CHECK: %cold
; CHECK: %coldlp
; CHECK: %hotlp
; CHECK: %lpret
entry:
%0 = icmp sgt i32 %a, 1
br i1 %0, label %hot, label %cold, !prof !4

hot:
%1 = invoke i32 @foo()
to label %then unwind label %hotlp

cold:
%2 = invoke i32 @bar()
to label %then unwind label %coldlp

then:
%3 = phi i32 [ %1, %hot ], [ %2, %cold ]
ret i32 %3

hotlp:
%4 = landingpad { i8*, i32 }
cleanup
br label %lpret

coldlp:
%5 = landingpad { i8*, i32 }
cleanup
br label %lpret

lpret:
%6 = phi i32 [-1, %hotlp], [-2, %coldlp]
%7 = add i32 %6, 42
ret i32 %7
}

!4 = !{!"branch_weights", i32 65536, i32 0}

; Make sure that ehpad are scheduled from the least probable one
; to the most probable one. See selectBestCandidateBlock as to why.
declare void @clean();

define void @test_flow_unwind() personality i32 (...)* @pers {
; CHECK-LABEL: test_flow_unwind:
; CHECK: %entry
; CHECK: %then
; CHECK: %exit
; CHECK: %innerlp
; CHECK: %outerlp
; CHECK: %outercleanup
entry:
%0 = invoke i32 @foo()
to label %then unwind label %outerlp

then:
%1 = invoke i32 @bar()
to label %exit unwind label %innerlp

exit:
ret void

innerlp:
%2 = landingpad { i8*, i32 }
cleanup
br label %innercleanup

outerlp:
%3 = landingpad { i8*, i32 }
cleanup
br label %outercleanup

outercleanup:
%4 = phi { i8*, i32 } [%2, %innercleanup], [%3, %outerlp]
call void @clean()
resume { i8*, i32 } %4

innercleanup:
call void @clean()
br label %outercleanup
}
4 changes: 2 additions & 2 deletions test/CodeGen/X86/seh-safe-div-win32.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,13 @@ __try.cont:

; Landing pad code

; CHECK: [[handler0:LBB0_[0-9]+]]: # %handler0
; CHECK: [[handler1:LBB0_[0-9]+]]: # %handler1
; Restore SP
; CHECK: movl {{.*}}(%ebp), %esp
; CHECK: calll _puts
; CHECK: jmp [[cont_bb]]

; CHECK: [[handler1:LBB0_[0-9]+]]: # %handler1
; CHECK: [[handler0:LBB0_[0-9]+]]: # %handler0
; Restore SP
; CHECK: movl {{.*}}(%ebp), %esp
; CHECK: calll _puts
Expand Down
8 changes: 4 additions & 4 deletions test/CodeGen/X86/seh-safe-div.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ __try.cont:

; Landing pad code

; CHECK: [[handler0:\.LBB0_[0-9]+]]: # %handler0
; CHECK: [[handler1:\.LBB0_[0-9]+]]: # %handler1
; CHECK: callq puts
; CHECK: movl $-1, [[rloc]]
; CHECK: movl $-2, [[rloc]]
; CHECK: jmp [[cont_bb]]

; CHECK: [[handler1:\.LBB0_[0-9]+]]: # %handler1
; CHECK: [[handler0:\.LBB0_[0-9]+]]: # %handler0
; CHECK: callq puts
; CHECK: movl $-2, [[rloc]]
; CHECK: movl $-1, [[rloc]]
; CHECK: jmp [[cont_bb]]

; CHECK: .seh_handlerdata
Expand Down
Loading

0 comments on commit a5bbcb5

Please sign in to comment.