From c3adcb034fe4dcf77749ea6db2ad0ba91a6fc01a Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Fri, 24 Sep 2021 13:21:45 -0400 Subject: [PATCH] Adce refactor (NFC) (#4547) * Have ADCE use cfg struct analysis (NFC) ADCE has a lot of code and variables to keep track of information that is easily obtains using the Struct cfg analysis. Most of this change is to refactor the code to have small functions to get the information from the struct cfg analysis. A few other changes small refactoring changes are done. * Factor out work list initialization in ADCE (NFC) We move the code that will initially populate the work list into its own function. We also simplify the code by making use of the struct cfg analysis. That way we can reduce the number of tables used to track information as we traverse the CFG. --- source/opt/aggressive_dead_code_elim_pass.cpp | 361 +++++++++--------- source/opt/aggressive_dead_code_elim_pass.h | 63 ++- 2 files changed, 202 insertions(+), 222 deletions(-) diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp index cb0000871..ac90fd343 100644 --- a/source/opt/aggressive_dead_code_elim_pass.cpp +++ b/source/opt/aggressive_dead_code_elim_pass.cpp @@ -35,7 +35,6 @@ namespace { const uint32_t kTypePointerStorageClassInIdx = 0; const uint32_t kEntryPointFunctionIdInIdx = 1; const uint32_t kSelectionMergeMergeBlockIdInIdx = 0; -const uint32_t kLoopMergeMergeBlockIdInIdx = 0; const uint32_t kLoopMergeContinueBlockIdInIdx = 1; const uint32_t kCopyMemoryTargetAddrInIdx = 0; const uint32_t kCopyMemorySourceAddrInIdx = 1; @@ -164,8 +163,7 @@ bool AggressiveDCEPass::AllExtensionsSupported() const { bool AggressiveDCEPass::IsDead(Instruction* inst) { if (IsLive(inst)) return false; if ((inst->IsBranch() || inst->opcode() == SpvOpUnreachable) && - !IsStructuredHeader(context()->get_instr_block(inst), nullptr, nullptr, - nullptr)) + context()->get_instr_block(inst)->GetMergeInst() == nullptr) return false; return true; } @@ -200,66 +198,6 @@ void AggressiveDCEPass::ProcessLoad(Function* func, uint32_t varId) { live_local_vars_.insert(varId); } -bool AggressiveDCEPass::IsStructuredHeader(BasicBlock* bp, - Instruction** mergeInst, - Instruction** branchInst, - uint32_t* mergeBlockId) { - if (!bp) return false; - Instruction* mi = bp->GetMergeInst(); - if (mi == nullptr) return false; - Instruction* bri = &*bp->tail(); - if (branchInst != nullptr) *branchInst = bri; - if (mergeInst != nullptr) *mergeInst = mi; - if (mergeBlockId != nullptr) *mergeBlockId = mi->GetSingleWordInOperand(0); - return true; -} - -void AggressiveDCEPass::ComputeBlock2HeaderMaps( - std::list& structuredOrder) { - block2headerBranch_.clear(); - header2nextHeaderBranch_.clear(); - branch2merge_.clear(); - structured_order_index_.clear(); - std::stack currentHeaderBranch; - currentHeaderBranch.push(nullptr); - uint32_t currentMergeBlockId = 0; - uint32_t index = 0; - for (auto bi = structuredOrder.begin(); bi != structuredOrder.end(); - ++bi, ++index) { - structured_order_index_[*bi] = index; - // If this block is the merge block of the current control construct, - // we are leaving the current construct so we must update state - if ((*bi)->id() == currentMergeBlockId) { - currentHeaderBranch.pop(); - Instruction* chb = currentHeaderBranch.top(); - if (chb != nullptr) - currentMergeBlockId = branch2merge_[chb]->GetSingleWordInOperand(0); - } - Instruction* mergeInst; - Instruction* branchInst; - uint32_t mergeBlockId; - bool is_header = - IsStructuredHeader(*bi, &mergeInst, &branchInst, &mergeBlockId); - // Map header block to next enclosing header. - if (is_header) header2nextHeaderBranch_[*bi] = currentHeaderBranch.top(); - // If this is a loop header, update state first so the block will map to - // itself. - if (is_header && mergeInst->opcode() == SpvOpLoopMerge) { - currentHeaderBranch.push(branchInst); - branch2merge_[branchInst] = mergeInst; - currentMergeBlockId = mergeBlockId; - } - // Map the block to the current construct. - block2headerBranch_[*bi] = currentHeaderBranch.top(); - // If this is an if header, update state so following blocks map to the if. - if (is_header && mergeInst->opcode() == SpvOpSelectionMerge) { - currentHeaderBranch.push(branchInst); - branch2merge_[branchInst] = mergeInst; - currentMergeBlockId = mergeBlockId; - } - } -} - void AggressiveDCEPass::AddBranch(uint32_t labelId, BasicBlock* bp) { std::unique_ptr newBranch( new Instruction(context(), SpvOpBranch, 0, 0, @@ -275,23 +213,18 @@ void AggressiveDCEPass::AddBreaksAndContinuesToWorklist( mergeInst->opcode() == SpvOpLoopMerge); BasicBlock* header = context()->get_instr_block(mergeInst); - uint32_t headerIndex = structured_order_index_[header]; const uint32_t mergeId = mergeInst->GetSingleWordInOperand(0); - BasicBlock* merge = context()->get_instr_block(mergeId); - uint32_t mergeIndex = structured_order_index_[merge]; - get_def_use_mgr()->ForEachUser( - mergeId, [headerIndex, mergeIndex, this](Instruction* user) { - if (!user->IsBranch()) return; - BasicBlock* block = context()->get_instr_block(user); - uint32_t index = structured_order_index_[block]; - if (headerIndex < index && index < mergeIndex) { - // This is a break from the loop. - AddToWorklist(user); - // Add branch's merge if there is one. - Instruction* userMerge = branch2merge_[user]; - if (userMerge != nullptr) AddToWorklist(userMerge); - } - }); + get_def_use_mgr()->ForEachUser(mergeId, [header, this](Instruction* user) { + if (!user->IsBranch()) return; + BasicBlock* block = context()->get_instr_block(user); + if (BlockIsInConstruct(header, block)) { + // This is a break from the loop. + AddToWorklist(user); + // Add branch's merge if there is one. + Instruction* userMerge = GetMergeInstruction(user); + if (userMerge != nullptr) AddToWorklist(userMerge); + } + }); if (mergeInst->opcode() != SpvOpLoopMerge) { return; @@ -305,7 +238,7 @@ void AggressiveDCEPass::AddBreaksAndContinuesToWorklist( if (op == SpvOpBranchConditional || op == SpvOpSwitch) { // A conditional branch or switch can only be a continue if it does not // have a merge instruction or its merge block is not the continue block. - Instruction* hdrMerge = branch2merge_[user]; + Instruction* hdrMerge = GetMergeInstruction(user); if (hdrMerge != nullptr && hdrMerge->opcode() == SpvOpSelectionMerge) { uint32_t hdrMergeId = hdrMerge->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx); @@ -317,9 +250,9 @@ void AggressiveDCEPass::AddBreaksAndContinuesToWorklist( // An unconditional branch can only be a continue if it is not // branching to its own merge block. BasicBlock* blk = context()->get_instr_block(user); - Instruction* hdrBranch = block2headerBranch_[blk]; + Instruction* hdrBranch = GetHeaderBranch(blk); if (hdrBranch == nullptr) return; - Instruction* hdrMerge = branch2merge_[hdrBranch]; + Instruction* hdrMerge = GetMergeInstruction(hdrBranch); if (hdrMerge->opcode() == SpvOpLoopMerge) return; uint32_t hdrMergeId = hdrMerge->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx); @@ -334,110 +267,14 @@ void AggressiveDCEPass::AddBreaksAndContinuesToWorklist( bool AggressiveDCEPass::AggressiveDCE(Function* func) { // Mark function parameters as live. AddToWorklist(&func->DefInst()); - func->ForEachParam( - [this](const Instruction* param) { - AddToWorklist(const_cast(param)); - }, - false); + MarkFunctionParameterAsLive(func); - // Compute map from block to controlling conditional branch std::list structuredOrder; cfg()->ComputeStructuredOrder(func, &*func->begin(), &structuredOrder); - ComputeBlock2HeaderMaps(structuredOrder); bool modified = false; - // Add instructions with external side effects to worklist. Also add branches - // EXCEPT those immediately contained in an "if" selection construct or a loop - // or continue construct. - // TODO(greg-lunarg): Handle Frexp, Modf more optimally - call_in_func_ = false; - func_is_entry_point_ = false; - private_stores_.clear(); live_local_vars_.clear(); - // Stacks to keep track of when we are inside an if- or loop-construct. - // When immediately inside an if- or loop-construct, we do not initially - // mark branches live. All other branches must be marked live. - std::stack assume_branches_live; - std::stack currentMergeBlockId; - // Push sentinel values on stack for when outside of any control flow. - assume_branches_live.push(true); - currentMergeBlockId.push(0); - for (auto bi = structuredOrder.begin(); bi != structuredOrder.end(); ++bi) { - // If exiting if or loop, update stacks - if ((*bi)->id() == currentMergeBlockId.top()) { - assume_branches_live.pop(); - currentMergeBlockId.pop(); - } - for (auto ii = (*bi)->begin(); ii != (*bi)->end(); ++ii) { - SpvOp op = ii->opcode(); - switch (op) { - case SpvOpStore: { - uint32_t varId; - (void)GetPtr(&*ii, &varId); - // Mark stores as live if their variable is not function scope - // and is not private scope. Remember private stores for possible - // later inclusion. We cannot call IsLocalVar at this point because - // private_like_local_ has not been set yet. - if (IsVarOfStorage(varId, SpvStorageClassPrivate) || - IsVarOfStorage(varId, SpvStorageClassWorkgroup)) - private_stores_.push_back(&*ii); - else if (!IsVarOfStorage(varId, SpvStorageClassFunction)) - AddToWorklist(&*ii); - } break; - case SpvOpCopyMemory: - case SpvOpCopyMemorySized: { - uint32_t varId; - (void)GetPtr(ii->GetSingleWordInOperand(kCopyMemoryTargetAddrInIdx), - &varId); - if (IsVarOfStorage(varId, SpvStorageClassPrivate) || - IsVarOfStorage(varId, SpvStorageClassWorkgroup)) - private_stores_.push_back(&*ii); - else if (!IsVarOfStorage(varId, SpvStorageClassFunction)) - AddToWorklist(&*ii); - } break; - case SpvOpLoopMerge: { - assume_branches_live.push(false); - currentMergeBlockId.push( - ii->GetSingleWordInOperand(kLoopMergeMergeBlockIdInIdx)); - } break; - case SpvOpSelectionMerge: { - assume_branches_live.push(false); - currentMergeBlockId.push( - ii->GetSingleWordInOperand(kSelectionMergeMergeBlockIdInIdx)); - } break; - case SpvOpSwitch: - case SpvOpBranch: - case SpvOpBranchConditional: - case SpvOpUnreachable: { - if (assume_branches_live.top()) { - AddToWorklist(&*ii); - } - } break; - default: { - // Function calls, atomics, function params, function returns, etc. - // TODO(greg-lunarg): function calls live only if write to non-local - if (!ii->IsOpcodeSafeToDelete()) { - AddToWorklist(&*ii); - } - // Remember function calls - if (op == SpvOpFunctionCall) call_in_func_ = true; - } break; - } - } - } - // See if current function is an entry point - for (auto& ei : get_module()->entry_points()) { - if (ei.GetSingleWordInOperand(kEntryPointFunctionIdInIdx) == - func->result_id()) { - func_is_entry_point_ = true; - break; - } - } - // If the current function is an entry point and has no function calls, - // we can optimize private variables as locals - private_like_local_ = func_is_entry_point_ && !call_in_func_; - // If privates are not like local, add their stores to worklist - if (!private_like_local_) - for (auto& ps : private_stores_) AddToWorklist(ps); + InitializeWorkList(func, structuredOrder); + // Perform closure on live instruction set. while (!worklist_.empty()) { Instruction* liveInst = worklist_.front(); @@ -469,18 +306,18 @@ bool AggressiveDCEPass::AggressiveDCE(Function* func) { // If in a structured if or loop construct, add the controlling // conditional branch and its merge. BasicBlock* blk = context()->get_instr_block(liveInst); - Instruction* branchInst = block2headerBranch_[blk]; + Instruction* branchInst = GetHeaderBranch(blk); if (branchInst != nullptr) { AddToWorklist(branchInst); - Instruction* mergeInst = branch2merge_[branchInst]; + Instruction* mergeInst = GetMergeInstruction(branchInst); AddToWorklist(mergeInst); } // If the block is a header, add the next outermost controlling // conditional branch and its merge. - Instruction* nextBranchInst = header2nextHeaderBranch_[blk]; + Instruction* nextBranchInst = GetBranchForNextHeader(blk); if (nextBranchInst != nullptr) { AddToWorklist(nextBranchInst); - Instruction* mergeInst = branch2merge_[nextBranchInst]; + Instruction* mergeInst = GetMergeInstruction(nextBranchInst); AddToWorklist(mergeInst); } // If local load, add all variable's stores if variable not already live @@ -621,6 +458,89 @@ bool AggressiveDCEPass::AggressiveDCE(Function* func) { return modified; } +void AggressiveDCEPass::InitializeWorkList( + Function* func, std::list& structuredOrder) { + // Add instructions with external side effects to the worklist. Also add + // branches that are not attached to a structured construct. + // TODO(s-perron): The handling of branch seems to be adhoc. This needs to be + // cleaned up. + bool call_in_func = false; + bool func_is_entry_point = false; + + // TODO(s-perron): We need to check if this is actually needed. In cases + // where private variable can be treated as if they are function scope, the + // private-to-local pass should be able to change them to function scope. + std::vector private_stores; + + for (auto bi = structuredOrder.begin(); bi != structuredOrder.end(); ++bi) { + for (auto ii = (*bi)->begin(); ii != (*bi)->end(); ++ii) { + SpvOp op = ii->opcode(); + switch (op) { + case SpvOpStore: { + uint32_t varId; + (void)GetPtr(&*ii, &varId); + // Mark stores as live if their variable is not function scope + // and is not private scope. Remember private stores for possible + // later inclusion. We cannot call IsLocalVar at this point because + // private_like_local_ has not been set yet. + if (IsVarOfStorage(varId, SpvStorageClassPrivate) || + IsVarOfStorage(varId, SpvStorageClassWorkgroup)) + private_stores.push_back(&*ii); + else if (!IsVarOfStorage(varId, SpvStorageClassFunction)) + AddToWorklist(&*ii); + } break; + case SpvOpCopyMemory: + case SpvOpCopyMemorySized: { + uint32_t varId; + (void)GetPtr(ii->GetSingleWordInOperand(kCopyMemoryTargetAddrInIdx), + &varId); + if (IsVarOfStorage(varId, SpvStorageClassPrivate) || + IsVarOfStorage(varId, SpvStorageClassWorkgroup)) + private_stores.push_back(&*ii); + else if (!IsVarOfStorage(varId, SpvStorageClassFunction)) + AddToWorklist(&*ii); + } break; + case SpvOpSwitch: + case SpvOpBranch: + case SpvOpBranchConditional: + case SpvOpUnreachable: { + bool branchRelatedToConstruct = + (GetMergeInstruction(&*ii) == nullptr && + GetHeaderBlock(context()->get_instr_block(&*ii)) == nullptr); + if (branchRelatedToConstruct) { + AddToWorklist(&*ii); + } + } break; + case SpvOpLoopMerge: + case SpvOpSelectionMerge: + break; + default: { + // Function calls, atomics, function params, function returns, etc. + if (!ii->IsOpcodeSafeToDelete()) { + AddToWorklist(&*ii); + } + // Remember function calls + if (op == SpvOpFunctionCall) call_in_func = true; + } break; + } + } + } + // See if current function is an entry point + for (auto& ei : get_module()->entry_points()) { + if (ei.GetSingleWordInOperand(kEntryPointFunctionIdInIdx) == + func->result_id()) { + func_is_entry_point = true; + break; + } + } + // If the current function is an entry point and has no function calls, + // we can optimize private variables as locals + private_like_local_ = func_is_entry_point && !call_in_func; + // If privates are not like local, add their stores to worklist + if (!private_like_local_) + for (auto& ps : private_stores) AddToWorklist(ps); +} + void AggressiveDCEPass::InitializeModuleScopeLiveInstructions() { // Keep all execution modes. for (auto& exec : get_module()->execution_modes()) { @@ -1027,5 +947,76 @@ void AggressiveDCEPass::InitExtensions() { }); } +Instruction* AggressiveDCEPass::GetHeaderBranch(BasicBlock* blk) { + if (blk == nullptr) { + return nullptr; + } + BasicBlock* header_block = GetHeaderBlock(blk); + if (header_block == nullptr) { + return nullptr; + } + return header_block->terminator(); +} + +BasicBlock* AggressiveDCEPass::GetHeaderBlock(BasicBlock* blk) const { + if (blk == nullptr) { + return nullptr; + } + + BasicBlock* header_block = nullptr; + if (blk->IsLoopHeader()) { + header_block = blk; + } else { + uint32_t header = + context()->GetStructuredCFGAnalysis()->ContainingConstruct(blk->id()); + header_block = context()->get_instr_block(header); + } + return header_block; +} + +Instruction* AggressiveDCEPass::GetMergeInstruction(Instruction* inst) { + BasicBlock* bb = context()->get_instr_block(inst); + if (bb == nullptr) { + return nullptr; + } + return bb->GetMergeInst(); +} + +Instruction* AggressiveDCEPass::GetBranchForNextHeader(BasicBlock* blk) { + if (blk == nullptr) { + return nullptr; + } + + if (blk->IsLoopHeader()) { + uint32_t header = + context()->GetStructuredCFGAnalysis()->ContainingConstruct(blk->id()); + blk = context()->get_instr_block(header); + } + return GetHeaderBranch(blk); +} + +void AggressiveDCEPass::MarkFunctionParameterAsLive(const Function* func) { + func->ForEachParam( + [this](const Instruction* param) { + AddToWorklist(const_cast(param)); + }, + false); +} + +bool AggressiveDCEPass::BlockIsInConstruct(BasicBlock* header_block, + BasicBlock* bb) { + if (bb == nullptr || header_block == nullptr) { + return false; + } + + uint32_t current_header = bb->id(); + while (current_header != 0) { + if (current_header == header_block->id()) return true; + current_header = context()->GetStructuredCFGAnalysis()->ContainingConstruct( + current_header); + } + return false; +} + } // namespace opt } // namespace spvtools diff --git a/source/opt/aggressive_dead_code_elim_pass.h b/source/opt/aggressive_dead_code_elim_pass.h index abc35895b..cd697b532 100644 --- a/source/opt/aggressive_dead_code_elim_pass.h +++ b/source/opt/aggressive_dead_code_elim_pass.h @@ -109,18 +109,6 @@ class AggressiveDCEPass : public MemPass { // If |varId| is local, mark all stores of varId as live. void ProcessLoad(Function* func, uint32_t varId); - // If |bp| is structured header block, returns true and sets |mergeInst| to - // the merge instruction, |branchInst| to the branch and |mergeBlockId| to the - // merge block if they are not nullptr. Any of |mergeInst|, |branchInst| or - // |mergeBlockId| may be a null pointer. Returns false if |bp| is a null - // pointer. - bool IsStructuredHeader(BasicBlock* bp, Instruction** mergeInst, - Instruction** branchInst, uint32_t* mergeBlockId); - - // Initialize block2headerBranch_, header2nextHeaderBranch_, and - // branch2merge_ using |structuredOrder| to order blocks. - void ComputeBlock2HeaderMaps(std::list& structuredOrder); - // Add branch to |labelId| to end of block |bp|. void AddBranch(uint32_t labelId, BasicBlock* bp); @@ -148,11 +136,33 @@ class AggressiveDCEPass : public MemPass { Pass::Status ProcessImpl(); - // True if current function has a call instruction contained in it - bool call_in_func_; + // Adds instructions which must be kept because of they have side-effects + // that ADCE cannot model to the work list. + void InitializeWorkList(Function* func, + std::list& structuredOrder); - // True if current function is an entry point - bool func_is_entry_point_; + // Marks all of the OpFunctionParameter instructions in |func| as live. + void MarkFunctionParameterAsLive(const Function* func); + + // Returns the terminator instruction in the header for the innermost + // construct that contains |blk|. Returns nullptr if no such header exists. + Instruction* GetHeaderBranch(BasicBlock* blk); + + // Returns the header for the innermost construct that contains |blk|. A loop + // header will be its own header. Returns nullptr if no such header exists. + BasicBlock* GetHeaderBlock(BasicBlock* blk) const; + + // Returns the same as |GetHeaderBlock| except if |blk| is a loop header it + // will return the header of the next enclosing construct. Returns nullptr if + // no such header exists. + Instruction* GetBranchForNextHeader(BasicBlock* blk); + + // Returns the merge instruction in the same basic block as |inst|. Returns + // nullptr if one does not exist. + Instruction* GetMergeInstruction(Instruction* inst); + + // Returns true if |bb| is in the construct with header |header_block|. + bool BlockIsInConstruct(BasicBlock* header_block, BasicBlock* bb); // True if current function is entry point and has no function calls. bool private_like_local_; @@ -164,27 +174,6 @@ class AggressiveDCEPass : public MemPass { // building up the live instructions set |live_insts_|. std::queue worklist_; - // Map from block to the branch instruction in the header of the most - // immediate controlling structured if or loop. A loop header block points - // to its own branch instruction. An if-selection block points to the branch - // of an enclosing construct's header, if one exists. - std::unordered_map block2headerBranch_; - - // Map from header block to the branch instruction in the header of the - // structured construct enclosing it. - // The liveness algorithm is designed to iteratively mark as live all - // structured constructs enclosing a live instruction. - std::unordered_map header2nextHeaderBranch_; - - // Maps basic block to their index in the structured order traversal. - std::unordered_map structured_order_index_; - - // Map from branch to its associated merge instruction, if any - std::unordered_map branch2merge_; - - // Store instructions to variables of private storage - std::vector private_stores_; - // Live Instructions utils::BitVector live_insts_;