From 56294b8af93400256e089ff52684f2e7bac36f1a Mon Sep 17 00:00:00 2001 From: Valery Pykhtin Date: Mon, 10 Mar 2025 09:22:48 +0000 Subject: [PATCH 1/3] [AMDGPU] Improve compile time of StructurizeCFG pass: avoid unnecessary DebugLoc map initialization. NFC. --- llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp index 89a2a7ac9be3f..db6321cc2f71e 100644 --- a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp +++ b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp @@ -361,6 +361,8 @@ class StructurizeCFG { void rebuildSSA(); + void setDebugLoc(BranchInst *Br, BasicBlock *BB); + public: void init(Region *R); bool run(Region *R, DominatorTree *DT); @@ -595,14 +597,6 @@ void StructurizeCFG::collectInfos() { // Find the last back edges analyzeLoops(RN); } - - // Reset the collected term debug locations - TermDL.clear(); - - for (BasicBlock &BB : *Func) { - if (const DebugLoc &DL = BB.getTerminator()->getDebugLoc()) - TermDL[&BB] = DL; - } } /// Insert the missing branch conditions @@ -924,12 +918,24 @@ void StructurizeCFG::simplifyAffectedPhis() { } while (Changed); } +void StructurizeCFG::setDebugLoc(BranchInst *Br, BasicBlock *BB) { + auto I = TermDL.find(BB); + if (I == TermDL.end()) + return; + + Br->setDebugLoc(I->second); + TermDL.erase(I); +} + /// Remove phi values from all successors and then remove the terminator. void StructurizeCFG::killTerminator(BasicBlock *BB) { Instruction *Term = BB->getTerminator(); if (!Term) return; + if (const DebugLoc &DL = Term->getDebugLoc()) + TermDL[BB] = DL; + for (BasicBlock *Succ : successors(BB)) delPhiValues(BB, Succ); @@ -974,7 +980,7 @@ void StructurizeCFG::changeExit(RegionNode *Node, BasicBlock *NewExit, BasicBlock *BB = Node->getNodeAs(); killTerminator(BB); BranchInst *Br = BranchInst::Create(NewExit, BB); - Br->setDebugLoc(TermDL[BB]); + setDebugLoc(Br, BB); addPhiValues(BB, NewExit); if (IncludeDominator) DT->changeImmediateDominator(NewExit, BB); @@ -990,10 +996,10 @@ BasicBlock *StructurizeCFG::getNextFlow(BasicBlock *Dominator) { Func, Insert); FlowSet.insert(Flow); - // use a temporary variable to avoid a use-after-free if the map's storage is - // reallocated - DebugLoc DL = TermDL[Dominator]; - TermDL[Flow] = std::move(DL); + auto *Term = Dominator->getTerminator(); + if (const DebugLoc &DL = + Term ? Term->getDebugLoc() : TermDL.lookup(Dominator)) + TermDL[Flow] = DL; DT->addNewBlock(Flow, Dominator); ParentRegion->getRegionInfo()->setRegionFor(Flow, ParentRegion); @@ -1088,7 +1094,7 @@ void StructurizeCFG::wireFlow(bool ExitUseAllowed, // let it point to entry and next block BranchInst *Br = BranchInst::Create(Entry, Next, BoolPoison, Flow); - Br->setDebugLoc(TermDL[Flow]); + setDebugLoc(Br, Flow); Conditions.push_back(Br); addPhiValues(Flow, Entry); DT->changeImmediateDominator(Entry, Flow); @@ -1129,7 +1135,7 @@ void StructurizeCFG::handleLoops(bool ExitUseAllowed, LoopEnd = needPrefix(false); BasicBlock *Next = needPostfix(LoopEnd, ExitUseAllowed); BranchInst *Br = BranchInst::Create(Next, LoopStart, BoolPoison, LoopEnd); - Br->setDebugLoc(TermDL[LoopEnd]); + setDebugLoc(Br, LoopEnd); LoopConds.push_back(Br); addPhiValues(LoopEnd, LoopStart); setPrevNode(Next); From b7ae8c8e9af6a59b2e579767cedb1d039fba55b0 Mon Sep 17 00:00:00 2001 From: Valery Pykhtin Date: Mon, 10 Mar 2025 15:24:11 +0000 Subject: [PATCH 2/3] Avoid using temporary DebugLoc object when possible. --- llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp index db6321cc2f71e..68547cd5f8246 100644 --- a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp +++ b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp @@ -996,10 +996,13 @@ BasicBlock *StructurizeCFG::getNextFlow(BasicBlock *Dominator) { Func, Insert); FlowSet.insert(Flow); - auto *Term = Dominator->getTerminator(); - if (const DebugLoc &DL = - Term ? Term->getDebugLoc() : TermDL.lookup(Dominator)) - TermDL[Flow] = DL; + if (auto *Term = Dominator->getTerminator()) { + if (const DebugLoc &DL = Term->getDebugLoc()) + TermDL[Flow] = DL; + } else if (DebugLoc DLTemp = TermDL.lookup(Dominator)) + // use a temporary variable to avoid a use-after-free if the map's storage + // is reallocated + TermDL[Flow] = DLTemp; DT->addNewBlock(Flow, Dominator); ParentRegion->getRegionInfo()->setRegionFor(Flow, ParentRegion); From e30c1cf907ee850aea5528a966866513d4f2c9bc Mon Sep 17 00:00:00 2001 From: Valery Pykhtin Date: Mon, 10 Mar 2025 18:02:09 +0000 Subject: [PATCH 3/3] Improved comment --- llvm/lib/Transforms/Scalar/StructurizeCFG.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp index 68547cd5f8246..a47beb6d7957b 100644 --- a/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp +++ b/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp @@ -999,10 +999,11 @@ BasicBlock *StructurizeCFG::getNextFlow(BasicBlock *Dominator) { if (auto *Term = Dominator->getTerminator()) { if (const DebugLoc &DL = Term->getDebugLoc()) TermDL[Flow] = DL; - } else if (DebugLoc DLTemp = TermDL.lookup(Dominator)) - // use a temporary variable to avoid a use-after-free if the map's storage - // is reallocated + } else if (DebugLoc DLTemp = TermDL.lookup(Dominator)) { + // Use a temporary copy to avoid a use-after-free if the map's storage + // is reallocated. TermDL[Flow] = DLTemp; + } DT->addNewBlock(Flow, Dominator); ParentRegion->getRegionInfo()->setRegionFor(Flow, ParentRegion);