Skip to content

Commit

Permalink
[WebAssembly] Misc. fixes in CFGStackify (NFC) (llvm#107182)
Browse files Browse the repository at this point in the history 
This contains misc. small fixes in CFGStackify. Most of them are comment
fixes and variable name changes. Two code changes are removing the cases
that can never occur. Another is extracting a routine as a lambda
function. I will add explanations inline in the code as Github comments.
  • Loading branch information
@aheejin
aheejin authored Sep 4, 2024
1 parent b2223b4 commit 32bc670
Showing 1 changed file with 67 additions and 74 deletions.
141 changes: 67 additions & 74 deletions llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass {
// over scoped regions when walking blocks.
SmallVector<MachineBasicBlock *, 8> ScopeTops;
void updateScopeTops(MachineBasicBlock *Begin, MachineBasicBlock *End) {
int BeginNo = Begin->getNumber();
int EndNo = End->getNumber();
if (!ScopeTops[EndNo] || ScopeTops[EndNo]->getNumber() > Begin->getNumber())
if (!ScopeTops[EndNo] || ScopeTops[EndNo]->getNumber() > BeginNo)
ScopeTops[EndNo] = Begin;
}

Expand All @@ -77,8 +78,8 @@ class WebAssemblyCFGStackify final : public MachineFunctionPass {
// Exception handling related functions
bool fixCallUnwindMismatches(MachineFunction &MF);
bool fixCatchUnwindMismatches(MachineFunction &MF);
void addTryDelegate(MachineInstr *RangeBegin, MachineInstr *RangeEnd,
MachineBasicBlock *DelegateDest);
void addNestedTryDelegate(MachineInstr *RangeBegin, MachineInstr *RangeEnd,
MachineBasicBlock *UnwindDest);
void recalculateScopeTops(MachineFunction &MF);
void removeUnnecessaryInstrs(MachineFunction &MF);

Expand Down Expand Up @@ -225,7 +226,7 @@ void WebAssemblyCFGStackify::registerScope(MachineInstr *Begin,
EndToBegin[End] = Begin;
}

// When 'End' is not an 'end_try' but 'delegate, EHPad is nullptr.
// When 'End' is not an 'end_try' but a 'delegate', EHPad is nullptr.
void WebAssemblyCFGStackify::registerTryScope(MachineInstr *Begin,
MachineInstr *End,
MachineBasicBlock *EHPad) {
Expand Down Expand Up @@ -293,7 +294,7 @@ void WebAssemblyCFGStackify::placeBlockMarker(MachineBasicBlock &MBB) {
}
}

// Decide where in Header to put the BLOCK.
// Decide where in MBB to put the BLOCK.

// Instructions that should go before the BLOCK.
SmallPtrSet<const MachineInstr *, 4> BeforeSet;
Expand Down Expand Up @@ -359,21 +360,20 @@ void WebAssemblyCFGStackify::placeBlockMarker(MachineBasicBlock &MBB) {
TII.get(WebAssembly::BLOCK))
.addImm(int64_t(ReturnType));

// Decide where in Header to put the END_BLOCK.
// Decide where in MBB to put the END_BLOCK.
BeforeSet.clear();
AfterSet.clear();
for (auto &MI : MBB) {
#ifndef NDEBUG
// END_BLOCK should precede existing LOOP and TRY markers.
if (MI.getOpcode() == WebAssembly::LOOP ||
MI.getOpcode() == WebAssembly::TRY)
// END_BLOCK should precede existing LOOP markers.
if (MI.getOpcode() == WebAssembly::LOOP)
AfterSet.insert(&MI);
#endif

// If there is a previously placed END_LOOP marker and the header of the
// loop is above this block's header, the END_LOOP should be placed after
// the BLOCK, because the loop contains this block. Otherwise the END_LOOP
// should be placed before the BLOCK. The same for END_TRY.
// the END_BLOCK, because the loop contains this block. Otherwise the
// END_LOOP should be placed before the END_BLOCK. The same for END_TRY.
if (MI.getOpcode() == WebAssembly::END_LOOP ||
MI.getOpcode() == WebAssembly::END_TRY) {
if (EndToBegin[&MI]->getParent()->getNumber() >= Header->getNumber())
Expand Down Expand Up @@ -437,7 +437,7 @@ void WebAssemblyCFGStackify::placeLoopMarker(MachineBasicBlock &MBB) {
TII.get(WebAssembly::LOOP))
.addImm(int64_t(WebAssembly::BlockType::Void));

// Decide where in Header to put the END_LOOP.
// Decide where in MBB to put the END_LOOP.
BeforeSet.clear();
AfterSet.clear();
#ifndef NDEBUG
Expand Down Expand Up @@ -491,20 +491,16 @@ void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) {
WebAssemblyException *WE = WEI.getExceptionFor(&MBB);
assert(WE);
MachineBasicBlock *Bottom = SRI.getBottom(WE);

auto Iter = std::next(Bottom->getIterator());
if (Iter == MF.end()) {
getAppendixBlock(MF);
Iter = std::next(Bottom->getIterator());
}
MachineBasicBlock *Cont = &*Iter;

assert(Cont != &MF.front());
MachineBasicBlock *LayoutPred = Cont->getPrevNode();

// If the nearest common dominator is inside a more deeply nested context,
// walk out to the nearest scope which isn't more deeply nested.
for (MachineFunction::iterator I(LayoutPred), E(Header); I != E; --I) {
for (MachineFunction::iterator I(Bottom), E(Header); I != E; --I) {
if (MachineBasicBlock *ScopeTop = ScopeTops[I->getNumber()]) {
if (ScopeTop->getNumber() > Header->getNumber()) {
// Skip over an intervening scope.
Expand Down Expand Up @@ -538,7 +534,7 @@ void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) {
}

// All previously inserted BLOCK/TRY markers should be after the TRY because
// they are all nested trys.
// they are all nested blocks/trys.
if (MI.getOpcode() == WebAssembly::BLOCK ||
MI.getOpcode() == WebAssembly::TRY)
AfterSet.insert(&MI);
Expand Down Expand Up @@ -607,14 +603,13 @@ void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) {
TII.get(WebAssembly::TRY))
.addImm(int64_t(WebAssembly::BlockType::Void));

// Decide where in Header to put the END_TRY.
// Decide where in Cont to put the END_TRY.
BeforeSet.clear();
AfterSet.clear();
for (const auto &MI : *Cont) {
#ifndef NDEBUG
// END_TRY should precede existing LOOP and BLOCK markers.
if (MI.getOpcode() == WebAssembly::LOOP ||
MI.getOpcode() == WebAssembly::BLOCK)
// END_TRY should precede existing LOOP markers.
if (MI.getOpcode() == WebAssembly::LOOP)
AfterSet.insert(&MI);

// All END_TRY markers placed earlier belong to exceptions that contains
Expand Down Expand Up @@ -643,9 +638,8 @@ void WebAssemblyCFGStackify::placeTryMarker(MachineBasicBlock &MBB) {

// Mark the end of the TRY.
InsertPos = getEarliestInsertPos(Cont, BeforeSet, AfterSet);
MachineInstr *End =
BuildMI(*Cont, InsertPos, Bottom->findBranchDebugLoc(),
TII.get(WebAssembly::END_TRY));
MachineInstr *End = BuildMI(*Cont, InsertPos, Bottom->findBranchDebugLoc(),
TII.get(WebAssembly::END_TRY));
registerTryScope(Begin, End, &MBB);

// Track the farthest-spanning scope that ends at this point. We create two
Expand Down Expand Up @@ -845,9 +839,9 @@ static void unstackifyVRegsUsedInSplitBB(MachineBasicBlock &MBB,

// Wrap the given range of instruction with try-delegate. RangeBegin and
// RangeEnd are inclusive.
void WebAssemblyCFGStackify::addTryDelegate(MachineInstr *RangeBegin,
MachineInstr *RangeEnd,
MachineBasicBlock *DelegateDest) {
void WebAssemblyCFGStackify::addNestedTryDelegate(
MachineInstr *RangeBegin, MachineInstr *RangeEnd,
MachineBasicBlock *UnwindDest) {
auto *BeginBB = RangeBegin->getParent();
auto *EndBB = RangeEnd->getParent();
MachineFunction &MF = *BeginBB->getParent();
Expand Down Expand Up @@ -879,8 +873,8 @@ void WebAssemblyCFGStackify::addTryDelegate(MachineInstr *RangeBegin,
MachineBasicBlock *DelegateBB = MF.CreateMachineBasicBlock();
// If the destination of 'delegate' is not the caller, adds the destination to
// the BB's successors.
if (DelegateDest != FakeCallerBB)
DelegateBB->addSuccessor(DelegateDest);
if (UnwindDest != FakeCallerBB)
DelegateBB->addSuccessor(UnwindDest);

auto SplitPos = std::next(RangeEnd->getIterator());
if (SplitPos == EndBB->end()) {
Expand Down Expand Up @@ -962,7 +956,7 @@ void WebAssemblyCFGStackify::addTryDelegate(MachineInstr *RangeBegin,
// Add 'delegate' instruction in the delegate BB created above.
MachineInstr *Delegate = BuildMI(DelegateBB, RangeEnd->getDebugLoc(),
TII.get(WebAssembly::DELEGATE))
.addMBB(DelegateDest);
.addMBB(UnwindDest);
registerTryScope(Try, Delegate, nullptr);
}

Expand Down Expand Up @@ -1130,7 +1124,7 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
if (EHPadStack.back() == UnwindDest)
continue;

// Include EH_LABELs in the range before and afer the invoke
// Include EH_LABELs in the range before and after the invoke
MachineInstr *RangeBegin = &MI, *RangeEnd = &MI;
if (RangeBegin->getIterator() != MBB.begin() &&
std::prev(RangeBegin->getIterator())->isEHLabel())
Expand Down Expand Up @@ -1231,22 +1225,24 @@ bool WebAssemblyCFGStackify::fixCallUnwindMismatches(MachineFunction &MF) {
std::tie(RangeBegin, RangeEnd) = Range;
auto *MBB = RangeBegin->getParent();

// If this BB has an EH pad successor, i.e., ends with an 'invoke', now we
// are going to wrap the invoke with try-delegate, making the 'delegate'
// BB the new successor instead, so remove the EH pad succesor here. The
// BB may not have an EH pad successor if calls in this BB throw to the
// caller.
MachineBasicBlock *EHPad = nullptr;
for (auto *Succ : MBB->successors()) {
if (Succ->isEHPad()) {
EHPad = Succ;
break;
// If this BB has an EH pad successor, i.e., ends with an 'invoke', and if
// the current range contains the invoke, now we are going to wrap the
// invoke with try-delegate, making the 'delegate' BB the new successor
// instead, so remove the EH pad succesor here. The BB may not have an EH
// pad successor if calls in this BB throw to the caller.
if (UnwindDest != getFakeCallerBlock(MF)) {
MachineBasicBlock *EHPad = nullptr;
for (auto *Succ : MBB->successors()) {
if (Succ->isEHPad()) {
EHPad = Succ;
break;
}
}
if (EHPad)
MBB->removeSuccessor(EHPad);
}
if (EHPad)
MBB->removeSuccessor(EHPad);

addTryDelegate(RangeBegin, RangeEnd, UnwindDest);
addNestedTryDelegate(RangeBegin, RangeEnd, UnwindDest);
}
}

Expand Down Expand Up @@ -1354,12 +1350,10 @@ bool WebAssemblyCFGStackify::fixCatchUnwindMismatches(MachineFunction &MF) {
NumCatchUnwindMismatches += EHPadToUnwindDest.size();
SmallPtrSet<MachineBasicBlock *, 4> NewEndTryBBs;

for (auto &P : EHPadToUnwindDest) {
MachineBasicBlock *EHPad = P.first;
MachineBasicBlock *UnwindDest = P.second;
for (auto &[EHPad, UnwindDest] : EHPadToUnwindDest) {
MachineInstr *Try = EHPadToTry[EHPad];
MachineInstr *EndTry = BeginToEnd[Try];
addTryDelegate(Try, EndTry, UnwindDest);
addNestedTryDelegate(Try, EndTry, UnwindDest);
NewEndTryBBs.insert(EndTry->getParent());
}

Expand Down Expand Up @@ -1534,7 +1528,7 @@ static void appendEndToFunction(MachineFunction &MF,
TII.get(WebAssembly::END_FUNCTION));
}

/// Insert LOOP/TRY/BLOCK markers at appropriate places.
/// Insert BLOCK/LOOP/TRY markers at appropriate places.
void WebAssemblyCFGStackify::placeMarkers(MachineFunction &MF) {
// We allocate one more than the number of blocks in the function to
// accommodate for the possible fake block we may insert at the end.
Expand All @@ -1558,9 +1552,9 @@ void WebAssemblyCFGStackify::placeMarkers(MachineFunction &MF) {
// Fix mismatches in unwind destinations induced by linearizing the code.
if (MCAI->getExceptionHandlingType() == ExceptionHandling::Wasm &&
MF.getFunction().hasPersonalityFn()) {
bool Changed = fixCallUnwindMismatches(MF);
Changed |= fixCatchUnwindMismatches(MF);
if (Changed)
bool MismatchFixed = fixCallUnwindMismatches(MF);
MismatchFixed |= fixCatchUnwindMismatches(MF);
if (MismatchFixed)
recalculateScopeTops(MF);
}
}
Expand Down Expand Up @@ -1654,6 +1648,23 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
// Now rewrite references to basic blocks to be depth immediates.
SmallVector<EndMarkerInfo, 8> Stack;
SmallVector<const MachineBasicBlock *, 8> EHPadStack;

auto RewriteOperands = [&](MachineInstr &MI) {
// Rewrite MBB operands to be depth immediates.
SmallVector<MachineOperand, 4> Ops(MI.operands());
while (MI.getNumOperands() > 0)
MI.removeOperand(MI.getNumOperands() - 1);
for (auto MO : Ops) {
if (MO.isMBB()) {
if (MI.getOpcode() == WebAssembly::DELEGATE)
MO = MachineOperand::CreateImm(getDelegateDepth(Stack, MO.getMBB()));
else
MO = MachineOperand::CreateImm(getBranchDepth(Stack, MO.getMBB()));
}
MI.addOperand(MF, MO);
}
};

for (auto &MBB : reverse(MF)) {
for (MachineInstr &MI : llvm::reverse(MBB)) {
switch (MI.getOpcode()) {
Expand Down Expand Up @@ -1697,23 +1708,8 @@ void WebAssemblyCFGStackify::rewriteDepthImmediates(MachineFunction &MF) {
break;

default:
if (MI.isTerminator()) {
// Rewrite MBB operands to be depth immediates.
SmallVector<MachineOperand, 4> Ops(MI.operands());
while (MI.getNumOperands() > 0)
MI.removeOperand(MI.getNumOperands() - 1);
for (auto MO : Ops) {
if (MO.isMBB()) {
if (MI.getOpcode() == WebAssembly::DELEGATE)
MO = MachineOperand::CreateImm(
getDelegateDepth(Stack, MO.getMBB()));
else
MO = MachineOperand::CreateImm(
getBranchDepth(Stack, MO.getMBB()));
}
MI.addOperand(MF, MO);
}
}
if (MI.isTerminator())
RewriteOperands(MI);

if (MI.getOpcode() == WebAssembly::DELEGATE)
Stack.push_back(std::make_pair(&MBB, &MI));
Expand Down Expand Up @@ -1767,10 +1763,7 @@ bool WebAssemblyCFGStackify::runOnMachineFunction(MachineFunction &MF) {

// Add an end instruction at the end of the function body.
const auto &TII = *MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
if (!MF.getSubtarget<WebAssemblySubtarget>()
.getTargetTriple()
.isOSBinFormatELF())
appendEndToFunction(MF, TII);
appendEndToFunction(MF, TII);

cleanupFunctionData(MF);

Expand Down

0 comments on commit 32bc670

Please sign in to comment.