From 2db7609b851d249e16a96cc40cdc7f710575b0bc Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Wed, 25 Dec 2024 02:30:22 +0000 Subject: [PATCH 01/23] Prevent invalid IR from being passed outside of RemoveDeadValues --- mlir/lib/Transforms/RemoveDeadValues.cpp | 206 +++++++++++++------ mlir/test/Transforms/remove-dead-values.mlir | 26 +++ 2 files changed, 172 insertions(+), 60 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 3429008b2f241..5d4ec66d6905a 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -72,15 +72,54 @@ using namespace mlir::dataflow; namespace { +// Set of structures below to be filled with operations and arguments to erase. +// This is done to separate analysis and tree modification phases, +// otherwise analysis is operating on half-deleted tree which is incorrect. + +struct CleanupFunction { + FunctionOpInterface funcOp; + BitVector nonLiveArgs; + BitVector nonLiveRets; +}; + +struct CleanupOperands { + Operation *op; + BitVector nonLiveOperands; +}; + +struct CleanupResults { + Operation *op; + BitVector nonLiveResults; +}; + +struct CleanupBlockArgs { + Block *b; + BitVector nonLiveArgs; +}; + +struct CleanupSuccessorOperands { + BranchOpInterface branch; + unsigned index; + BitVector nonLiveOperands; +}; + +struct CleanupList { + SmallVector operations; + SmallVector values; + SmallVector functions; + SmallVector operands; + SmallVector results; + SmallVector blocks; + SmallVector successorOperands; +}; + // Some helper functions... /// Return true iff at least one value in `values` is live, given the liveness /// information in `la`. -static bool hasLive(ValueRange values, RunLivenessAnalysis &la) { +static bool hasLive(ValueRange values, const DenseSet &deletionSet, RunLivenessAnalysis &la) { for (Value value : values) { - // If there is a null value, it implies that it was dropped during the - // execution of this pass, implying that it was non-live. - if (!value) + if (deletionSet.contains(value)) continue; const Liveness *liveness = la.getLiveness(value); @@ -92,11 +131,11 @@ static bool hasLive(ValueRange values, RunLivenessAnalysis &la) { /// Return a BitVector of size `values.size()` where its i-th bit is 1 iff the /// i-th value in `values` is live, given the liveness information in `la`. -static BitVector markLives(ValueRange values, RunLivenessAnalysis &la) { +static BitVector markLives(ValueRange values, const DenseSet &deletionSet, RunLivenessAnalysis &la) { BitVector lives(values.size(), true); for (auto [index, value] : llvm::enumerate(values)) { - if (!value) { + if (deletionSet.contains(value)) { lives.reset(index); continue; } @@ -115,6 +154,18 @@ static BitVector markLives(ValueRange values, RunLivenessAnalysis &la) { return lives; } +// DeletionSet is used to track the Values that are scheduled for removal +void updateDeletionSet(DenseSet &deletionSet, ValueRange range, const BitVector &nonLive) { + for (auto [index, result] : llvm::enumerate(range)) { + if (!nonLive[index]) continue; + deletionSet.insert(result); + } +} + +void updateDeletionSet(DenseSet &deletionSet, Operation *op, const BitVector &nonLive) { + updateDeletionSet(deletionSet, op->getResults(), nonLive); +} + /// Drop the uses of the i-th result of `op` and then erase it iff toErase[i] /// is 1. static void dropUsesAndEraseResults(Operation *op, BitVector toErase) { @@ -174,43 +225,43 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// It is assumed that `op` is simple. Here, a simple op is one which isn't a /// function-like op, a call-like op, a region branch op, a branch op, a region /// branch terminator op, or return-like. -static void cleanSimpleOp(Operation *op, RunLivenessAnalysis &la) { - if (!isMemoryEffectFree(op) || hasLive(op->getResults(), la)) +static void cleanSimpleOp(CleanupList &cl, DenseSet &deletionSet, Operation *op, RunLivenessAnalysis &la) { + if (!isMemoryEffectFree(op) || hasLive(op->getResults(), deletionSet, la)) return; - op->dropAllUses(); - op->erase(); + cl.operations.push_back(op); + updateDeletionSet(deletionSet, op, BitVector(op->getNumResults(), true)); } /// Clean a function-like op `funcOp`, given the liveness information in `la` /// and the IR in `module`. Here, cleaning means: /// (1) Dropping the uses of its unnecessary (non-live) arguments, -/// (2) Erasing these arguments, -/// (3) Erasing their corresponding operands from its callers, +/// (2) Erasing their corresponding operands from its callers, +/// (3) Erasing these arguments, /// (4) Erasing its unnecessary terminator operands (return values that are /// non-live across all callers), /// (5) Dropping the uses of these return values from its callers, AND /// (6) Erasing these return values /// iff it is not public or external. -static void cleanFuncOp(FunctionOpInterface funcOp, Operation *module, +static void cleanFuncOp(CleanupList &cl, DenseSet &deletionSet, + FunctionOpInterface funcOp, Operation *module, RunLivenessAnalysis &la) { if (funcOp.isPublic() || funcOp.isExternal()) return; // Get the list of unnecessary (non-live) arguments in `nonLiveArgs`. SmallVector arguments(funcOp.getArguments()); - BitVector nonLiveArgs = markLives(arguments, la); + BitVector nonLiveArgs = markLives(arguments, deletionSet, la); nonLiveArgs = nonLiveArgs.flip(); // Do (1). for (auto [index, arg] : llvm::enumerate(arguments)) - if (arg && nonLiveArgs[index]) - arg.dropAllUses(); + if (arg && nonLiveArgs[index]) { + cl.values.push_back(arg); + deletionSet.insert(arg); + } // Do (2). - funcOp.eraseArguments(nonLiveArgs); - - // Do (3). SymbolTable::UseRange uses = *funcOp.getSymbolUses(module); for (SymbolTable::SymbolUse use : uses) { Operation *callOp = use.getUser(); @@ -222,7 +273,7 @@ static void cleanFuncOp(FunctionOpInterface funcOp, Operation *module, operandsToOpOperands(cast(callOp).getArgOperands()); for (int index : nonLiveArgs.set_bits()) nonLiveCallOperands.set(callOpOperands[index]->getOperandNumber()); - callOp->eraseOperands(nonLiveCallOperands); + cl.operands.push_back({callOp, nonLiveCallOperands}); } // Get the list of unnecessary terminator operands (return values that are @@ -253,26 +304,27 @@ static void cleanFuncOp(FunctionOpInterface funcOp, Operation *module, for (SymbolTable::SymbolUse use : uses) { Operation *callOp = use.getUser(); assert(isa(callOp) && "expected a call-like user"); - BitVector liveCallRets = markLives(callOp->getResults(), la); + BitVector liveCallRets = markLives(callOp->getResults(), deletionSet, la); nonLiveRets &= liveCallRets.flip(); } - // Do (4). + // Do (3). // Note that in the absence of control flow ops forcing the control to go from // the entry (first) block to the other blocks, the control never reaches any // block other than the entry block, because every block has a terminator. for (Block &block : funcOp.getBlocks()) { Operation *returnOp = block.getTerminator(); if (returnOp && returnOp->getNumOperands() == numReturns) - returnOp->eraseOperands(nonLiveRets); + cl.operands.push_back({returnOp, nonLiveRets}); } - funcOp.eraseResults(nonLiveRets); + cl.functions.push_back({funcOp, nonLiveArgs, nonLiveRets}); // Do (5) and (6). for (SymbolTable::SymbolUse use : uses) { Operation *callOp = use.getUser(); assert(isa(callOp) && "expected a call-like user"); - dropUsesAndEraseResults(callOp, nonLiveRets); + cl.results.push_back({callOp, nonLiveRets}); + updateDeletionSet(deletionSet, callOp, nonLiveRets); } } @@ -297,18 +349,19 @@ static void cleanFuncOp(FunctionOpInterface funcOp, Operation *module, /// It is important to note that values in this op flow from operands and /// terminator operands (successor operands) to arguments and results (successor /// inputs). -static void cleanRegionBranchOp(RegionBranchOpInterface regionBranchOp, +static void cleanRegionBranchOp(CleanupList &cl, DenseSet &deletionSet, + RegionBranchOpInterface regionBranchOp, RunLivenessAnalysis &la) { // Mark live results of `regionBranchOp` in `liveResults`. auto markLiveResults = [&](BitVector &liveResults) { - liveResults = markLives(regionBranchOp->getResults(), la); + liveResults = markLives(regionBranchOp->getResults(), deletionSet, la); }; // Mark live arguments in the regions of `regionBranchOp` in `liveArgs`. auto markLiveArgs = [&](DenseMap &liveArgs) { for (Region ®ion : regionBranchOp->getRegions()) { SmallVector arguments(region.front().getArguments()); - BitVector regionLiveArgs = markLives(arguments, la); + BitVector regionLiveArgs = markLives(arguments, deletionSet, la); liveArgs[®ion] = regionLiveArgs; } }; @@ -497,9 +550,8 @@ static void cleanRegionBranchOp(RegionBranchOpInterface regionBranchOp, // It could never be live because of this op but its liveness could have been // attributed to something else. if (isMemoryEffectFree(regionBranchOp.getOperation()) && - !hasLive(regionBranchOp->getResults(), la)) { - regionBranchOp->dropAllUses(); - regionBranchOp->erase(); + !hasLive(regionBranchOp->getResults(), deletionSet, la)) { + cl.operations.push_back(regionBranchOp.getOperation()); return; } @@ -538,29 +590,27 @@ static void cleanRegionBranchOp(RegionBranchOpInterface regionBranchOp, terminatorOperandsToKeep); // Do (1). - regionBranchOp->eraseOperands(operandsToKeep.flip()); + cl.operands.push_back({regionBranchOp, operandsToKeep.flip()}); // Do (2.a) and (2.b). for (Region ®ion : regionBranchOp->getRegions()) { assert(!region.empty() && "expected a non-empty region in an op " "implementing `RegionBranchOpInterface`"); - for (auto [index, arg] : llvm::enumerate(region.front().getArguments())) { - if (argsToKeep[®ion][index]) - continue; - if (arg) - arg.dropAllUses(); - } - region.front().eraseArguments(argsToKeep[®ion].flip()); + BitVector argsToRemove = argsToKeep[®ion].flip(); + cl.blocks.push_back({®ion.front(), argsToRemove}); + updateDeletionSet(deletionSet, region.front().getArguments(), argsToRemove); } // Do (2.c). for (Region ®ion : regionBranchOp->getRegions()) { Operation *terminator = region.front().getTerminator(); - terminator->eraseOperands(terminatorOperandsToKeep[terminator].flip()); + cl.operands.push_back({terminator, terminatorOperandsToKeep[terminator].flip()}); } // Do (3) and (4). - dropUsesAndEraseResults(regionBranchOp.getOperation(), resultsToKeep.flip()); + BitVector resultsToRemove = resultsToKeep.flip(); + updateDeletionSet(deletionSet, regionBranchOp.getOperation(), resultsToRemove); + cl.results.push_back({regionBranchOp.getOperation(), resultsToRemove}); } // 1. Iterate over each successor block of the given BranchOpInterface @@ -572,7 +622,8 @@ static void cleanRegionBranchOp(RegionBranchOpInterface regionBranchOp, // c. Mark each operand as live or dead based on the analysis. // 3. Remove dead operands from the branch operation and arguments accordingly -static void cleanBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la) { +static void cleanBranchOp(CleanupList &cl, DenseSet &deletionSet, + BranchOpInterface branchOp, RunLivenessAnalysis &la) { unsigned numSuccessors = branchOp->getNumSuccessors(); // Do (1) @@ -588,22 +639,53 @@ static void cleanBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la) { operandValues.push_back(successorOperands[operandIdx]); } - BitVector successorLiveOperands = markLives(operandValues, la); - // Do (3) - for (int argIdx = successorLiveOperands.size() - 1; argIdx >= 0; --argIdx) { - if (!successorLiveOperands[argIdx]) { - if (successorBlock->getNumArguments() < successorOperands.size()) { - // if block was cleaned through a different code path - // we only need to remove operands from the invokation - successorOperands.erase(argIdx); - continue; - } + BitVector successorNonLive = markLives(operandValues, deletionSet, la).flip(); + updateDeletionSet(deletionSet, successorBlock->getArguments(), successorNonLive); + cl.blocks.push_back({successorBlock, successorNonLive}); + cl.successorOperands.push_back({branchOp, succIdx, successorNonLive}); + } +} + +void cleanup(CleanupList &cl) { + for (auto &op: cl.operations) { + op->dropAllUses(); + op->erase(); + } + + for (auto &v: cl.values) { + v.dropAllUses(); + } + + for (auto &f: cl.functions) { + f.funcOp.eraseArguments(f.nonLiveArgs); + f.funcOp.eraseResults(f.nonLiveRets); + } + + for (auto &o: cl.operands) { + o.op->eraseOperands(o.nonLiveOperands); } + + for (auto &r: cl.results) { + dropUsesAndEraseResults(r.op, r.nonLiveResults); + } - successorBlock->getArgument(argIdx).dropAllUses(); - successorOperands.erase(argIdx); - successorBlock->eraseArgument(argIdx); - } + for (auto &b: cl.blocks) { + // blocks that are accessed via multiple codepaths processed once + if (b.b->getNumArguments() != b.nonLiveArgs.size()) continue; + for (int i = b.nonLiveArgs.size() - 1; i >= 0; --i) { + if (!b.nonLiveArgs[i]) continue; + b.b->getArgument(i).dropAllUses(); + b.b->eraseArgument(i); + } + } + for (auto &op: cl.successorOperands) { + SuccessorOperands successorOperands = + op.branch.getSuccessorOperands(op.index); + // blocks that are accessed via multiple codepaths processed once + if (successorOperands.size() != op.nonLiveOperands.size()) continue; + for (int i = successorOperands.size() - 1; i >= 0; --i) { + if (!op.nonLiveOperands[i]) continue; + successorOperands.erase(i); } } } @@ -616,14 +698,16 @@ struct RemoveDeadValues : public impl::RemoveDeadValuesBase { void RemoveDeadValues::runOnOperation() { auto &la = getAnalysis(); Operation *module = getOperation(); + DenseSet deletionSet; + CleanupList cl; module->walk([&](Operation *op) { if (auto funcOp = dyn_cast(op)) { - cleanFuncOp(funcOp, module, la); + cleanFuncOp(cl, deletionSet, funcOp, module, la); } else if (auto regionBranchOp = dyn_cast(op)) { - cleanRegionBranchOp(regionBranchOp, la); + cleanRegionBranchOp(cl, deletionSet, regionBranchOp, la); } else if (auto branchOp = dyn_cast(op)) { - cleanBranchOp(branchOp, la); + cleanBranchOp(cl, deletionSet, branchOp, la); } else if (op->hasTrait<::mlir::OpTrait::IsTerminator>()) { // Nothing to do here because this is a terminator op and it should be // honored with respect to its parent @@ -631,9 +715,11 @@ void RemoveDeadValues::runOnOperation() { // Nothing to do because this op is associated with a function op and gets // cleaned when the latter is cleaned. } else { - cleanSimpleOp(op, la); + cleanSimpleOp(cl, deletionSet, op, la); } }); + + cleanup(cl); } std::unique_ptr mlir::createRemoveDeadValuesPass() { diff --git a/mlir/test/Transforms/remove-dead-values.mlir b/mlir/test/Transforms/remove-dead-values.mlir index 9273ac01e7cce..fe7bcbc7c490b 100644 --- a/mlir/test/Transforms/remove-dead-values.mlir +++ b/mlir/test/Transforms/remove-dead-values.mlir @@ -73,6 +73,32 @@ func.func @cleanable_loop_iter_args_value(%arg0: index) -> index { // ----- +// Checking that the arguments of linalg.generic are properly handled +// All code below is removed as a result of the pass +// +#map = affine_map<(d0, d1, d2) -> (0, d1, d2)> +#map1 = affine_map<(d0, d1, d2) -> (d0, d1, d2)> +module { + func.func @main() { + %cst_3 = arith.constant dense<54> : tensor<1x25x13xi32> + %cst_7 = arith.constant dense<11> : tensor<1x25x13xi32> + // CHECK-NOT: arith.constant + %0 = tensor.empty() : tensor<1x25x13xi32> + // CHECK-NOT: tensor + %1 = linalg.generic {indexing_maps = [#map, #map, #map1], iterator_types = ["parallel", "parallel", "parallel"]} ins(%cst_3, %cst_7 : tensor<1x25x13xi32>, tensor<1x25x13xi32>) outs(%0 : tensor<1x25x13xi32>) { + // CHECK-NOT: linalg.generic + ^bb0(%in: i32, %in_15: i32, %out: i32): + %29 = arith.xori %in, %in_15 : i32 + // CHECK-NOT: arith.xori + linalg.yield %29 : i32 + // CHECK-NOT: linalg.yield + } -> tensor<1x25x13xi32> + return + } +} + +// ----- + // Note that this cleanup cannot be done by the `canonicalize` pass. // // CHECK-LABEL: func.func private @clean_func_op_remove_argument_and_return_value() { From 38edd5a7780da99c24716d4a3c0d8436cfdb8b7a Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Wed, 25 Dec 2024 02:47:15 +0000 Subject: [PATCH 02/23] Update formatting --- mlir/lib/Transforms/RemoveDeadValues.cpp | 62 +++++++++++++++--------- 1 file changed, 39 insertions(+), 23 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 5d4ec66d6905a..b30b883ebf230 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -117,7 +117,8 @@ struct CleanupList { /// Return true iff at least one value in `values` is live, given the liveness /// information in `la`. -static bool hasLive(ValueRange values, const DenseSet &deletionSet, RunLivenessAnalysis &la) { +static bool hasLive(ValueRange values, const DenseSet &deletionSet, + RunLivenessAnalysis &la) { for (Value value : values) { if (deletionSet.contains(value)) continue; @@ -131,7 +132,9 @@ static bool hasLive(ValueRange values, const DenseSet &deletionSet, RunLi /// Return a BitVector of size `values.size()` where its i-th bit is 1 iff the /// i-th value in `values` is live, given the liveness information in `la`. -static BitVector markLives(ValueRange values, const DenseSet &deletionSet, RunLivenessAnalysis &la) { +static BitVector markLives(ValueRange values, + const DenseSet &deletionSet, + RunLivenessAnalysis &la) { BitVector lives(values.size(), true); for (auto [index, value] : llvm::enumerate(values)) { @@ -155,14 +158,17 @@ static BitVector markLives(ValueRange values, const DenseSet &deletionSet } // DeletionSet is used to track the Values that are scheduled for removal -void updateDeletionSet(DenseSet &deletionSet, ValueRange range, const BitVector &nonLive) { +void updateDeletionSet(DenseSet &deletionSet, ValueRange range, + const BitVector &nonLive) { for (auto [index, result] : llvm::enumerate(range)) { - if (!nonLive[index]) continue; + if (!nonLive[index]) + continue; deletionSet.insert(result); } } -void updateDeletionSet(DenseSet &deletionSet, Operation *op, const BitVector &nonLive) { +void updateDeletionSet(DenseSet &deletionSet, Operation *op, + const BitVector &nonLive) { updateDeletionSet(deletionSet, op->getResults(), nonLive); } @@ -225,7 +231,8 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// It is assumed that `op` is simple. Here, a simple op is one which isn't a /// function-like op, a call-like op, a region branch op, a branch op, a region /// branch terminator op, or return-like. -static void cleanSimpleOp(CleanupList &cl, DenseSet &deletionSet, Operation *op, RunLivenessAnalysis &la) { +static void cleanSimpleOp(CleanupList &cl, DenseSet &deletionSet, + Operation *op, RunLivenessAnalysis &la) { if (!isMemoryEffectFree(op) || hasLive(op->getResults(), deletionSet, la)) return; @@ -604,12 +611,14 @@ static void cleanRegionBranchOp(CleanupList &cl, DenseSet &deletionSet, // Do (2.c). for (Region ®ion : regionBranchOp->getRegions()) { Operation *terminator = region.front().getTerminator(); - cl.operands.push_back({terminator, terminatorOperandsToKeep[terminator].flip()}); + cl.operands.push_back( + {terminator, terminatorOperandsToKeep[terminator].flip()}); } // Do (3) and (4). BitVector resultsToRemove = resultsToKeep.flip(); - updateDeletionSet(deletionSet, regionBranchOp.getOperation(), resultsToRemove); + updateDeletionSet(deletionSet, regionBranchOp.getOperation(), + resultsToRemove); cl.results.push_back({regionBranchOp.getOperation(), resultsToRemove}); } @@ -640,51 +649,58 @@ static void cleanBranchOp(CleanupList &cl, DenseSet &deletionSet, } // Do (3) - BitVector successorNonLive = markLives(operandValues, deletionSet, la).flip(); - updateDeletionSet(deletionSet, successorBlock->getArguments(), successorNonLive); + BitVector successorNonLive = + markLives(operandValues, deletionSet, la).flip(); + updateDeletionSet(deletionSet, successorBlock->getArguments(), + successorNonLive); cl.blocks.push_back({successorBlock, successorNonLive}); cl.successorOperands.push_back({branchOp, succIdx, successorNonLive}); } } void cleanup(CleanupList &cl) { - for (auto &op: cl.operations) { + for (auto &op : cl.operations) { op->dropAllUses(); op->erase(); } - for (auto &v: cl.values) { + for (auto &v : cl.values) { v.dropAllUses(); } - for (auto &f: cl.functions) { + for (auto &f : cl.functions) { f.funcOp.eraseArguments(f.nonLiveArgs); f.funcOp.eraseResults(f.nonLiveRets); } - for (auto &o: cl.operands) { - o.op->eraseOperands(o.nonLiveOperands); } + for (auto &o : cl.operands) { + o.op->eraseOperands(o.nonLiveOperands); + } - for (auto &r: cl.results) { + for (auto &r : cl.results) { dropUsesAndEraseResults(r.op, r.nonLiveResults); } - for (auto &b: cl.blocks) { + for (auto &b : cl.blocks) { // blocks that are accessed via multiple codepaths processed once - if (b.b->getNumArguments() != b.nonLiveArgs.size()) continue; + if (b.b->getNumArguments() != b.nonLiveArgs.size()) + continue; for (int i = b.nonLiveArgs.size() - 1; i >= 0; --i) { - if (!b.nonLiveArgs[i]) continue; + if (!b.nonLiveArgs[i]) + continue; b.b->getArgument(i).dropAllUses(); b.b->eraseArgument(i); } } - for (auto &op: cl.successorOperands) { + for (auto &op : cl.successorOperands) { SuccessorOperands successorOperands = - op.branch.getSuccessorOperands(op.index); + op.branch.getSuccessorOperands(op.index); // blocks that are accessed via multiple codepaths processed once - if (successorOperands.size() != op.nonLiveOperands.size()) continue; + if (successorOperands.size() != op.nonLiveOperands.size()) + continue; for (int i = successorOperands.size() - 1; i >= 0; --i) { - if (!op.nonLiveOperands[i]) continue; + if (!op.nonLiveOperands[i]) + continue; successorOperands.erase(i); } } From ff9a4c107d5294d2c95b05593971e7b8db47fb1e Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Tue, 31 Dec 2024 16:14:14 +0000 Subject: [PATCH 03/23] Addressing review feedback --- mlir/lib/Transforms/RemoveDeadValues.cpp | 93 +++++++++++++----------- 1 file changed, 49 insertions(+), 44 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index b30b883ebf230..6167e4b4426b7 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -82,14 +82,9 @@ struct CleanupFunction { BitVector nonLiveRets; }; -struct CleanupOperands { +struct CleanupOperation { Operation *op; - BitVector nonLiveOperands; -}; - -struct CleanupResults { - Operation *op; - BitVector nonLiveResults; + BitVector nonLive; }; struct CleanupBlockArgs { @@ -99,7 +94,7 @@ struct CleanupBlockArgs { struct CleanupSuccessorOperands { BranchOpInterface branch; - unsigned index; + unsigned successorIndex; BitVector nonLiveOperands; }; @@ -107,8 +102,8 @@ struct CleanupList { SmallVector operations; SmallVector values; SmallVector functions; - SmallVector operands; - SmallVector results; + SmallVector operands; + SmallVector results; SmallVector blocks; SmallVector successorOperands; }; @@ -157,8 +152,10 @@ static BitVector markLives(ValueRange values, return lives; } -// DeletionSet is used to track the Values that are scheduled for removal -void updateDeletionSet(DenseSet &deletionSet, ValueRange range, +/// Collects values marked as "non-live" in the provided range and inserts them +/// into the given set. A value is considered "non-live" if the corresponding +/// index in the `nonLive` bit vector is set. +static void collectNonLiveValues(DenseSet &deletionSet, ValueRange range, const BitVector &nonLive) { for (auto [index, result] : llvm::enumerate(range)) { if (!nonLive[index]) @@ -167,11 +164,6 @@ void updateDeletionSet(DenseSet &deletionSet, ValueRange range, } } -void updateDeletionSet(DenseSet &deletionSet, Operation *op, - const BitVector &nonLive) { - updateDeletionSet(deletionSet, op->getResults(), nonLive); -} - /// Drop the uses of the i-th result of `op` and then erase it iff toErase[i] /// is 1. static void dropUsesAndEraseResults(Operation *op, BitVector toErase) { @@ -231,28 +223,28 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// It is assumed that `op` is simple. Here, a simple op is one which isn't a /// function-like op, a call-like op, a region branch op, a branch op, a region /// branch terminator op, or return-like. -static void cleanSimpleOp(CleanupList &cl, DenseSet &deletionSet, - Operation *op, RunLivenessAnalysis &la) { +static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, + DenseSet &deletionSet, CleanupList &cl) { if (!isMemoryEffectFree(op) || hasLive(op->getResults(), deletionSet, la)) return; cl.operations.push_back(op); - updateDeletionSet(deletionSet, op, BitVector(op->getNumResults(), true)); + collectNonLiveValues(deletionSet, op->getResults(), BitVector(op->getNumResults(), true)); } /// Clean a function-like op `funcOp`, given the liveness information in `la` /// and the IR in `module`. Here, cleaning means: /// (1) Dropping the uses of its unnecessary (non-live) arguments, /// (2) Erasing their corresponding operands from its callers, -/// (3) Erasing these arguments, -/// (4) Erasing its unnecessary terminator operands (return values that are +/// (3) Erasing its unnecessary terminator operands (return values that are /// non-live across all callers), +/// (4) Erasing these arguments, /// (5) Dropping the uses of these return values from its callers, AND /// (6) Erasing these return values /// iff it is not public or external. -static void cleanFuncOp(CleanupList &cl, DenseSet &deletionSet, - FunctionOpInterface funcOp, Operation *module, - RunLivenessAnalysis &la) { +static void processFuncOp(FunctionOpInterface funcOp, Operation *module, + RunLivenessAnalysis &la, + DenseSet &deletionSet, CleanupList &cl) { if (funcOp.isPublic() || funcOp.isExternal()) return; @@ -283,6 +275,7 @@ static void cleanFuncOp(CleanupList &cl, DenseSet &deletionSet, cl.operands.push_back({callOp, nonLiveCallOperands}); } + // Do (3). // Get the list of unnecessary terminator operands (return values that are // non-live across all callers) in `nonLiveRets`. There is a very important // subtlety here. Unnecessary terminator operands are NOT the operands of the @@ -315,7 +308,7 @@ static void cleanFuncOp(CleanupList &cl, DenseSet &deletionSet, nonLiveRets &= liveCallRets.flip(); } - // Do (3). + // Do (4). // Note that in the absence of control flow ops forcing the control to go from // the entry (first) block to the other blocks, the control never reaches any // block other than the entry block, because every block has a terminator. @@ -331,7 +324,7 @@ static void cleanFuncOp(CleanupList &cl, DenseSet &deletionSet, Operation *callOp = use.getUser(); assert(isa(callOp) && "expected a call-like user"); cl.results.push_back({callOp, nonLiveRets}); - updateDeletionSet(deletionSet, callOp, nonLiveRets); + collectNonLiveValues(deletionSet, callOp->getResults(), nonLiveRets); } } @@ -356,9 +349,9 @@ static void cleanFuncOp(CleanupList &cl, DenseSet &deletionSet, /// It is important to note that values in this op flow from operands and /// terminator operands (successor operands) to arguments and results (successor /// inputs). -static void cleanRegionBranchOp(CleanupList &cl, DenseSet &deletionSet, - RegionBranchOpInterface regionBranchOp, - RunLivenessAnalysis &la) { +static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, + RunLivenessAnalysis &la, + DenseSet &deletionSet, CleanupList &cl) { // Mark live results of `regionBranchOp` in `liveResults`. auto markLiveResults = [&](BitVector &liveResults) { liveResults = markLives(regionBranchOp->getResults(), deletionSet, la); @@ -605,7 +598,7 @@ static void cleanRegionBranchOp(CleanupList &cl, DenseSet &deletionSet, "implementing `RegionBranchOpInterface`"); BitVector argsToRemove = argsToKeep[®ion].flip(); cl.blocks.push_back({®ion.front(), argsToRemove}); - updateDeletionSet(deletionSet, region.front().getArguments(), argsToRemove); + collectNonLiveValues(deletionSet, region.front().getArguments(), argsToRemove); } // Do (2.c). @@ -617,7 +610,7 @@ static void cleanRegionBranchOp(CleanupList &cl, DenseSet &deletionSet, // Do (3) and (4). BitVector resultsToRemove = resultsToKeep.flip(); - updateDeletionSet(deletionSet, regionBranchOp.getOperation(), + collectNonLiveValues(deletionSet, regionBranchOp.getOperation()->getResults(), resultsToRemove); cl.results.push_back({regionBranchOp.getOperation(), resultsToRemove}); } @@ -631,8 +624,8 @@ static void cleanRegionBranchOp(CleanupList &cl, DenseSet &deletionSet, // c. Mark each operand as live or dead based on the analysis. // 3. Remove dead operands from the branch operation and arguments accordingly -static void cleanBranchOp(CleanupList &cl, DenseSet &deletionSet, - BranchOpInterface branchOp, RunLivenessAnalysis &la) { +static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, + DenseSet &deletionSet, CleanupList &cl) { unsigned numSuccessors = branchOp->getNumSuccessors(); // Do (1) @@ -651,36 +644,42 @@ static void cleanBranchOp(CleanupList &cl, DenseSet &deletionSet, // Do (3) BitVector successorNonLive = markLives(operandValues, deletionSet, la).flip(); - updateDeletionSet(deletionSet, successorBlock->getArguments(), + collectNonLiveValues(deletionSet, successorBlock->getArguments(), successorNonLive); cl.blocks.push_back({successorBlock, successorNonLive}); cl.successorOperands.push_back({branchOp, succIdx, successorNonLive}); } } -void cleanup(CleanupList &cl) { +static void cleanUpDeadVals(CleanupList &cl) { + // 1. Operations for (auto &op : cl.operations) { op->dropAllUses(); op->erase(); } + // 2. Values for (auto &v : cl.values) { v.dropAllUses(); } + // 3. Functions for (auto &f : cl.functions) { f.funcOp.eraseArguments(f.nonLiveArgs); f.funcOp.eraseResults(f.nonLiveRets); } + // 4. Operands for (auto &o : cl.operands) { - o.op->eraseOperands(o.nonLiveOperands); + o.op->eraseOperands(o.nonLive); } + // 5. Results for (auto &r : cl.results) { - dropUsesAndEraseResults(r.op, r.nonLiveResults); + dropUsesAndEraseResults(r.op, r.nonLive); } + // 6. Blocks for (auto &b : cl.blocks) { // blocks that are accessed via multiple codepaths processed once if (b.b->getNumArguments() != b.nonLiveArgs.size()) @@ -692,9 +691,11 @@ void cleanup(CleanupList &cl) { b.b->eraseArgument(i); } } + + // 7. Successor Operands for (auto &op : cl.successorOperands) { SuccessorOperands successorOperands = - op.branch.getSuccessorOperands(op.index); + op.branch.getSuccessorOperands(op.successorIndex); // blocks that are accessed via multiple codepaths processed once if (successorOperands.size() != op.nonLiveOperands.size()) continue; @@ -714,16 +715,20 @@ struct RemoveDeadValues : public impl::RemoveDeadValuesBase { void RemoveDeadValues::runOnOperation() { auto &la = getAnalysis(); Operation *module = getOperation(); + + // Tracks values eligible for erasure - complements liveness analysis to identify "droppable" values. DenseSet deletionSet; + + // Maintains a list of Ops, values, branches, etc., slated for cleanup at the end of this pass. CleanupList cl; module->walk([&](Operation *op) { if (auto funcOp = dyn_cast(op)) { - cleanFuncOp(cl, deletionSet, funcOp, module, la); + processFuncOp(funcOp, module, la, deletionSet, cl); } else if (auto regionBranchOp = dyn_cast(op)) { - cleanRegionBranchOp(cl, deletionSet, regionBranchOp, la); + processRegionBranchOp(regionBranchOp, la, deletionSet, cl); } else if (auto branchOp = dyn_cast(op)) { - cleanBranchOp(cl, deletionSet, branchOp, la); + processBranchOp(branchOp, la, deletionSet, cl); } else if (op->hasTrait<::mlir::OpTrait::IsTerminator>()) { // Nothing to do here because this is a terminator op and it should be // honored with respect to its parent @@ -731,11 +736,11 @@ void RemoveDeadValues::runOnOperation() { // Nothing to do because this op is associated with a function op and gets // cleaned when the latter is cleaned. } else { - cleanSimpleOp(cl, deletionSet, op, la); + processSimpleOp(op, la, deletionSet, cl); } }); - cleanup(cl); + cleanUpDeadVals(cl); } std::unique_ptr mlir::createRemoveDeadValuesPass() { From 97a219c8683a110f6a807886603ee9c545cb8132 Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Tue, 31 Dec 2024 16:15:24 +0000 Subject: [PATCH 04/23] Formatting --- mlir/lib/Transforms/RemoveDeadValues.cpp | 27 ++++++++++++++---------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 6167e4b4426b7..fa265e67c0424 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -156,7 +156,7 @@ static BitVector markLives(ValueRange values, /// into the given set. A value is considered "non-live" if the corresponding /// index in the `nonLive` bit vector is set. static void collectNonLiveValues(DenseSet &deletionSet, ValueRange range, - const BitVector &nonLive) { + const BitVector &nonLive) { for (auto [index, result] : llvm::enumerate(range)) { if (!nonLive[index]) continue; @@ -224,12 +224,13 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// function-like op, a call-like op, a region branch op, a branch op, a region /// branch terminator op, or return-like. static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, - DenseSet &deletionSet, CleanupList &cl) { + DenseSet &deletionSet, CleanupList &cl) { if (!isMemoryEffectFree(op) || hasLive(op->getResults(), deletionSet, la)) return; cl.operations.push_back(op); - collectNonLiveValues(deletionSet, op->getResults(), BitVector(op->getNumResults(), true)); + collectNonLiveValues(deletionSet, op->getResults(), + BitVector(op->getNumResults(), true)); } /// Clean a function-like op `funcOp`, given the liveness information in `la` @@ -243,8 +244,8 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, /// (6) Erasing these return values /// iff it is not public or external. static void processFuncOp(FunctionOpInterface funcOp, Operation *module, - RunLivenessAnalysis &la, - DenseSet &deletionSet, CleanupList &cl) { + RunLivenessAnalysis &la, DenseSet &deletionSet, + CleanupList &cl) { if (funcOp.isPublic() || funcOp.isExternal()) return; @@ -351,7 +352,8 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, /// inputs). static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, RunLivenessAnalysis &la, - DenseSet &deletionSet, CleanupList &cl) { + DenseSet &deletionSet, + CleanupList &cl) { // Mark live results of `regionBranchOp` in `liveResults`. auto markLiveResults = [&](BitVector &liveResults) { liveResults = markLives(regionBranchOp->getResults(), deletionSet, la); @@ -598,7 +600,8 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, "implementing `RegionBranchOpInterface`"); BitVector argsToRemove = argsToKeep[®ion].flip(); cl.blocks.push_back({®ion.front(), argsToRemove}); - collectNonLiveValues(deletionSet, region.front().getArguments(), argsToRemove); + collectNonLiveValues(deletionSet, region.front().getArguments(), + argsToRemove); } // Do (2.c). @@ -611,7 +614,7 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, // Do (3) and (4). BitVector resultsToRemove = resultsToKeep.flip(); collectNonLiveValues(deletionSet, regionBranchOp.getOperation()->getResults(), - resultsToRemove); + resultsToRemove); cl.results.push_back({regionBranchOp.getOperation(), resultsToRemove}); } @@ -645,7 +648,7 @@ static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, BitVector successorNonLive = markLives(operandValues, deletionSet, la).flip(); collectNonLiveValues(deletionSet, successorBlock->getArguments(), - successorNonLive); + successorNonLive); cl.blocks.push_back({successorBlock, successorNonLive}); cl.successorOperands.push_back({branchOp, succIdx, successorNonLive}); } @@ -716,10 +719,12 @@ void RemoveDeadValues::runOnOperation() { auto &la = getAnalysis(); Operation *module = getOperation(); - // Tracks values eligible for erasure - complements liveness analysis to identify "droppable" values. + // Tracks values eligible for erasure - complements liveness analysis to + // identify "droppable" values. DenseSet deletionSet; - // Maintains a list of Ops, values, branches, etc., slated for cleanup at the end of this pass. + // Maintains a list of Ops, values, branches, etc., slated for cleanup at the + // end of this pass. CleanupList cl; module->walk([&](Operation *op) { From 3af2d06bbd6d127fd28a3192ae50f9452bccf6bc Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Wed, 1 Jan 2025 19:18:57 +0000 Subject: [PATCH 05/23] Addressing review feedback --- mlir/lib/Transforms/RemoveDeadValues.cpp | 149 ++++++++++++----------- 1 file changed, 77 insertions(+), 72 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index fa265e67c0424..f031e03577fb4 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -76,23 +76,23 @@ namespace { // This is done to separate analysis and tree modification phases, // otherwise analysis is operating on half-deleted tree which is incorrect. -struct CleanupFunction { +struct FunctionToCleanUp { FunctionOpInterface funcOp; BitVector nonLiveArgs; BitVector nonLiveRets; }; -struct CleanupOperation { +struct OperationToCleanup { Operation *op; BitVector nonLive; }; -struct CleanupBlockArgs { +struct BlockArgsToCleanup { Block *b; BitVector nonLiveArgs; }; -struct CleanupSuccessorOperands { +struct SuccessorOperandsToCleanup { BranchOpInterface branch; unsigned successorIndex; BitVector nonLiveOperands; @@ -101,11 +101,11 @@ struct CleanupSuccessorOperands { struct CleanupList { SmallVector operations; SmallVector values; - SmallVector functions; - SmallVector operands; - SmallVector results; - SmallVector blocks; - SmallVector successorOperands; + SmallVector functions; + SmallVector operands; + SmallVector results; + SmallVector blocks; + SmallVector successorOperands; }; // Some helper functions... @@ -214,15 +214,19 @@ static SmallVector operandsToOpOperands(OperandRange operands) { return opOperands; } -/// Clean a simple op `op`, given the liveness analysis information in `la`. -/// Here, cleaning means: -/// (1) Dropping all its uses, AND -/// (2) Erasing it -/// iff it has no memory effects and none of its results are live. +/// Process a simple operation `op` using the liveness analysis `la`. +/// If the operation has no memory effects and none of its results are live: +/// 1. Adding the operation to a list for future removal, and +/// 2. Marking all its results as non-live values /// -/// It is assumed that `op` is simple. Here, a simple op is one which isn't a -/// function-like op, a call-like op, a region branch op, a branch op, a region -/// branch terminator op, or return-like. +/// The operation `op` is assumed to be simple. +/// A simple operation is one that is NOT: +/// - Function-like +/// - Call-like +/// - A region branch operation +/// - A branch operation +/// - A region branch terminator +/// - Return-like static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, DenseSet &deletionSet, CleanupList &cl) { if (!isMemoryEffectFree(op) || hasLive(op->getResults(), deletionSet, la)) @@ -233,16 +237,15 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, BitVector(op->getNumResults(), true)); } -/// Clean a function-like op `funcOp`, given the liveness information in `la` -/// and the IR in `module`. Here, cleaning means: -/// (1) Dropping the uses of its unnecessary (non-live) arguments, -/// (2) Erasing their corresponding operands from its callers, -/// (3) Erasing its unnecessary terminator operands (return values that are -/// non-live across all callers), -/// (4) Erasing these arguments, -/// (5) Dropping the uses of these return values from its callers, AND -/// (6) Erasing these return values -/// iff it is not public or external. +/// Process a function-like operation `funcOp` using the liveness analysis `la` +/// and the IR in `module`. If it is not public or external: +/// 1. Adding its non-live arguments to a list for future removal. +/// 2. Marking their corresponding operands in its callers for removal. +/// 3. Identifying and enqueueing unnecessary terminator operands +/// (return values that are non-live across all callers) for removal. +/// 4. Enqueueing the non-live arguments themselves for removal. +/// 5. Collecting the uses of these return values in its callers for future removal. +/// 6. Marking all its results as non-live values. static void processFuncOp(FunctionOpInterface funcOp, Operation *module, RunLivenessAnalysis &la, DenseSet &deletionSet, CleanupList &cl) { @@ -329,27 +332,30 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, } } -/// Clean a region branch op `regionBranchOp`, given the liveness information in -/// `la`. Here, cleaning means: -/// (1') Dropping all its uses, AND -/// (2') Erasing it -/// if it has no memory effects and none of its results are live, AND -/// (1) Erasing its unnecessary operands (operands that are forwarded to -/// unneccesary results and arguments), -/// (2) Cleaning each of its regions, -/// (3) Dropping the uses of its unnecessary results (results that are -/// forwarded from unnecessary operands and terminator operands), AND -/// (4) Erasing these results -/// otherwise. -/// Note that here, cleaning a region means: -/// (2.a) Dropping the uses of its unnecessary arguments (arguments that are -/// forwarded from unneccesary operands and terminator operands), -/// (2.b) Erasing these arguments, AND -/// (2.c) Erasing its unnecessary terminator operands (terminator operands -/// that are forwarded to unneccesary results and arguments). -/// It is important to note that values in this op flow from operands and -/// terminator operands (successor operands) to arguments and results (successor -/// inputs). +/// Process a region branch operation `regionBranchOp` using the liveness information in `la`. +/// The processing involves two scenarios: +/// +/// Scenario 1: If the operation has no memory effects and none of its results are live: +/// 1. Enqueue all its uses for deletion. +/// 2. Enqueue the branch itself for deletion. +/// +/// Scenario 2: Otherwise: +/// 1. Collect its unnecessary operands (operands forwarded to unnecessary results or arguments). +/// 2. Process each of its regions. +/// 3. Collect the uses of its unnecessary results (results forwarded from unnecessary operands +/// or terminator operands). +/// 4. Add these results to the deletion list. +/// +/// Processing a region includes: +/// a. Collecting the uses of its unnecessary arguments (arguments forwarded from unnecessary operands +/// or terminator operands). +/// b. Collecting these unnecessary arguments. +/// c. Collecting its unnecessary terminator operands (terminator operands forwarded to unnecessary results +/// or arguments). +/// +/// Value Flow Note: In this operation, values flow as follows: +/// - From operands and terminator operands (successor operands) +/// - To arguments and results (successor inputs). static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, RunLivenessAnalysis &la, DenseSet &deletionSet, @@ -546,7 +552,7 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, } }; - // Do (1') and (2'). This is the only case where the entire `regionBranchOp` + // Scenario 1. This is the only case where the entire `regionBranchOp` // is removed. It will not happen in any other scenario. Note that in this // case, a non-forwarded operand of `regionBranchOp` could be live/non-live. // It could never be live because of this op but its liveness could have been @@ -557,6 +563,7 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, return; } + // Scenario 2. // At this point, we know that every non-forwarded operand of `regionBranchOp` // is live. @@ -618,14 +625,12 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, cl.results.push_back({regionBranchOp.getOperation(), resultsToRemove}); } -// 1. Iterate over each successor block of the given BranchOpInterface -// operation. -// 2. For each successor block: -// a. Retrieve the operands passed to the successor. -// b. Use the provided liveness analysis (`RunLivenessAnalysis`) to determine -// which operands are live in the successor block. -// c. Mark each operand as live or dead based on the analysis. -// 3. Remove dead operands from the branch operation and arguments accordingly +/// Steps to process a `BranchOpInterface` operation: +/// 1. Iterate through each successor block of the operation. +/// 2. For each successor block, gather all operands from all successors +/// along with their associated liveness analysis data. +/// 3. Identify and collect the dead operands from the branch operation +/// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, DenseSet &deletionSet, CleanupList &cl) { @@ -654,36 +659,36 @@ static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, } } -static void cleanUpDeadVals(CleanupList &cl) { +static void cleanUpDeadVals(CleanupList &cleanupList) { // 1. Operations - for (auto &op : cl.operations) { + for (auto &op : cleanupList.operations) { op->dropAllUses(); op->erase(); } // 2. Values - for (auto &v : cl.values) { + for (auto &v : cleanupList.values) { v.dropAllUses(); } // 3. Functions - for (auto &f : cl.functions) { + for (auto &f : cleanupList.functions) { f.funcOp.eraseArguments(f.nonLiveArgs); f.funcOp.eraseResults(f.nonLiveRets); } // 4. Operands - for (auto &o : cl.operands) { + for (auto &o : cleanupList.operands) { o.op->eraseOperands(o.nonLive); } // 5. Results - for (auto &r : cl.results) { + for (auto &r : cleanupList.results) { dropUsesAndEraseResults(r.op, r.nonLive); } // 6. Blocks - for (auto &b : cl.blocks) { + for (auto &b : cleanupList.blocks) { // blocks that are accessed via multiple codepaths processed once if (b.b->getNumArguments() != b.nonLiveArgs.size()) continue; @@ -696,7 +701,7 @@ static void cleanUpDeadVals(CleanupList &cl) { } // 7. Successor Operands - for (auto &op : cl.successorOperands) { + for (auto &op : cleanupList.successorOperands) { SuccessorOperands successorOperands = op.branch.getSuccessorOperands(op.successorIndex); // blocks that are accessed via multiple codepaths processed once @@ -721,19 +726,19 @@ void RemoveDeadValues::runOnOperation() { // Tracks values eligible for erasure - complements liveness analysis to // identify "droppable" values. - DenseSet deletionSet; + DenseSet deadVals; // Maintains a list of Ops, values, branches, etc., slated for cleanup at the // end of this pass. - CleanupList cl; + CleanupList finalCleanUpList; module->walk([&](Operation *op) { if (auto funcOp = dyn_cast(op)) { - processFuncOp(funcOp, module, la, deletionSet, cl); + processFuncOp(funcOp, module, la, deadVals, finalCleanUpList); } else if (auto regionBranchOp = dyn_cast(op)) { - processRegionBranchOp(regionBranchOp, la, deletionSet, cl); + processRegionBranchOp(regionBranchOp, la, deadVals, finalCleanUpList); } else if (auto branchOp = dyn_cast(op)) { - processBranchOp(branchOp, la, deletionSet, cl); + processBranchOp(branchOp, la, deadVals, finalCleanUpList); } else if (op->hasTrait<::mlir::OpTrait::IsTerminator>()) { // Nothing to do here because this is a terminator op and it should be // honored with respect to its parent @@ -741,11 +746,11 @@ void RemoveDeadValues::runOnOperation() { // Nothing to do because this op is associated with a function op and gets // cleaned when the latter is cleaned. } else { - processSimpleOp(op, la, deletionSet, cl); + processSimpleOp(op, la, deadVals, finalCleanUpList); } }); - cleanUpDeadVals(cl); + cleanUpDeadVals(finalCleanUpList); } std::unique_ptr mlir::createRemoveDeadValuesPass() { From 506a3dc1eaef9a168b176ad431acd427b8f3fb52 Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Wed, 1 Jan 2025 19:19:57 +0000 Subject: [PATCH 06/23] Formatting --- mlir/lib/Transforms/RemoveDeadValues.cpp | 34 ++++++++++++++---------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index f031e03577fb4..0140e4fa7304f 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -219,7 +219,7 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// 1. Adding the operation to a list for future removal, and /// 2. Marking all its results as non-live values /// -/// The operation `op` is assumed to be simple. +/// The operation `op` is assumed to be simple. /// A simple operation is one that is NOT: /// - Function-like /// - Call-like @@ -237,14 +237,15 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, BitVector(op->getNumResults(), true)); } -/// Process a function-like operation `funcOp` using the liveness analysis `la` +/// Process a function-like operation `funcOp` using the liveness analysis `la` /// and the IR in `module`. If it is not public or external: /// 1. Adding its non-live arguments to a list for future removal. /// 2. Marking their corresponding operands in its callers for removal. -/// 3. Identifying and enqueueing unnecessary terminator operands +/// 3. Identifying and enqueueing unnecessary terminator operands /// (return values that are non-live across all callers) for removal. /// 4. Enqueueing the non-live arguments themselves for removal. -/// 5. Collecting the uses of these return values in its callers for future removal. +/// 5. Collecting the uses of these return values in its callers for future +/// removal. /// 6. Marking all its results as non-live values. static void processFuncOp(FunctionOpInterface funcOp, Operation *module, RunLivenessAnalysis &la, DenseSet &deletionSet, @@ -332,29 +333,34 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, } } -/// Process a region branch operation `regionBranchOp` using the liveness information in `la`. -/// The processing involves two scenarios: +/// Process a region branch operation `regionBranchOp` using the liveness +/// information in `la`. The processing involves two scenarios: /// -/// Scenario 1: If the operation has no memory effects and none of its results are live: +/// Scenario 1: If the operation has no memory effects and none of its results +/// are live: /// 1. Enqueue all its uses for deletion. /// 2. Enqueue the branch itself for deletion. /// /// Scenario 2: Otherwise: -/// 1. Collect its unnecessary operands (operands forwarded to unnecessary results or arguments). +/// 1. Collect its unnecessary operands (operands forwarded to unnecessary +/// results or arguments). /// 2. Process each of its regions. -/// 3. Collect the uses of its unnecessary results (results forwarded from unnecessary operands +/// 3. Collect the uses of its unnecessary results (results forwarded from +/// unnecessary operands /// or terminator operands). /// 4. Add these results to the deletion list. /// /// Processing a region includes: -/// a. Collecting the uses of its unnecessary arguments (arguments forwarded from unnecessary operands +/// a. Collecting the uses of its unnecessary arguments (arguments forwarded +/// from unnecessary operands /// or terminator operands). /// b. Collecting these unnecessary arguments. -/// c. Collecting its unnecessary terminator operands (terminator operands forwarded to unnecessary results +/// c. Collecting its unnecessary terminator operands (terminator operands +/// forwarded to unnecessary results /// or arguments). /// /// Value Flow Note: In this operation, values flow as follows: -/// - From operands and terminator operands (successor operands) +/// - From operands and terminator operands (successor operands) /// - To arguments and results (successor inputs). static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, RunLivenessAnalysis &la, @@ -627,9 +633,9 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, /// Steps to process a `BranchOpInterface` operation: /// 1. Iterate through each successor block of the operation. -/// 2. For each successor block, gather all operands from all successors +/// 2. For each successor block, gather all operands from all successors /// along with their associated liveness analysis data. -/// 3. Identify and collect the dead operands from the branch operation +/// 3. Identify and collect the dead operands from the branch operation /// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, From 3aac5135bbc61f63de168364567ce63b960b139e Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Wed, 1 Jan 2025 19:24:31 +0000 Subject: [PATCH 07/23] Addressing review feedback --- mlir/lib/Transforms/RemoveDeadValues.cpp | 38 ++++++++++++------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 0140e4fa7304f..8869ad48f7d2d 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -98,7 +98,7 @@ struct SuccessorOperandsToCleanup { BitVector nonLiveOperands; }; -struct CleanupList { +struct RDVFinalCleanupList { SmallVector operations; SmallVector values; SmallVector functions; @@ -228,7 +228,7 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// - A region branch terminator /// - Return-like static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, - DenseSet &deletionSet, CleanupList &cl) { + DenseSet &deletionSet, RDVFinalCleanupList &cl) { if (!isMemoryEffectFree(op) || hasLive(op->getResults(), deletionSet, la)) return; @@ -249,7 +249,7 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, /// 6. Marking all its results as non-live values. static void processFuncOp(FunctionOpInterface funcOp, Operation *module, RunLivenessAnalysis &la, DenseSet &deletionSet, - CleanupList &cl) { + RDVFinalCleanupList &cl) { if (funcOp.isPublic() || funcOp.isExternal()) return; @@ -365,7 +365,7 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, RunLivenessAnalysis &la, DenseSet &deletionSet, - CleanupList &cl) { + RDVFinalCleanupList &cl) { // Mark live results of `regionBranchOp` in `liveResults`. auto markLiveResults = [&](BitVector &liveResults) { liveResults = markLives(regionBranchOp->getResults(), deletionSet, la); @@ -639,7 +639,7 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, /// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, - DenseSet &deletionSet, CleanupList &cl) { + DenseSet &deletionSet, RDVFinalCleanupList &cl) { unsigned numSuccessors = branchOp->getNumSuccessors(); // Do (1) @@ -665,36 +665,36 @@ static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, } } -static void cleanUpDeadVals(CleanupList &cleanupList) { +static void cleanUpDeadVals(RDVFinalCleanupList &RDVFinalCleanupList) { // 1. Operations - for (auto &op : cleanupList.operations) { + for (auto &op : RDVFinalCleanupList.operations) { op->dropAllUses(); op->erase(); } // 2. Values - for (auto &v : cleanupList.values) { + for (auto &v : RDVFinalCleanupList.values) { v.dropAllUses(); } // 3. Functions - for (auto &f : cleanupList.functions) { + for (auto &f : RDVFinalCleanupList.functions) { f.funcOp.eraseArguments(f.nonLiveArgs); f.funcOp.eraseResults(f.nonLiveRets); } // 4. Operands - for (auto &o : cleanupList.operands) { + for (auto &o : RDVFinalCleanupList.operands) { o.op->eraseOperands(o.nonLive); } // 5. Results - for (auto &r : cleanupList.results) { + for (auto &r : RDVFinalCleanupList.results) { dropUsesAndEraseResults(r.op, r.nonLive); } // 6. Blocks - for (auto &b : cleanupList.blocks) { + for (auto &b : RDVFinalCleanupList.blocks) { // blocks that are accessed via multiple codepaths processed once if (b.b->getNumArguments() != b.nonLiveArgs.size()) continue; @@ -707,7 +707,7 @@ static void cleanUpDeadVals(CleanupList &cleanupList) { } // 7. Successor Operands - for (auto &op : cleanupList.successorOperands) { + for (auto &op : RDVFinalCleanupList.successorOperands) { SuccessorOperands successorOperands = op.branch.getSuccessorOperands(op.successorIndex); // blocks that are accessed via multiple codepaths processed once @@ -736,15 +736,15 @@ void RemoveDeadValues::runOnOperation() { // Maintains a list of Ops, values, branches, etc., slated for cleanup at the // end of this pass. - CleanupList finalCleanUpList; + RDVFinalCleanupList finalRDVFinalCleanupList; module->walk([&](Operation *op) { if (auto funcOp = dyn_cast(op)) { - processFuncOp(funcOp, module, la, deadVals, finalCleanUpList); + processFuncOp(funcOp, module, la, deadVals, finalRDVFinalCleanupList); } else if (auto regionBranchOp = dyn_cast(op)) { - processRegionBranchOp(regionBranchOp, la, deadVals, finalCleanUpList); + processRegionBranchOp(regionBranchOp, la, deadVals, finalRDVFinalCleanupList); } else if (auto branchOp = dyn_cast(op)) { - processBranchOp(branchOp, la, deadVals, finalCleanUpList); + processBranchOp(branchOp, la, deadVals, finalRDVFinalCleanupList); } else if (op->hasTrait<::mlir::OpTrait::IsTerminator>()) { // Nothing to do here because this is a terminator op and it should be // honored with respect to its parent @@ -752,11 +752,11 @@ void RemoveDeadValues::runOnOperation() { // Nothing to do because this op is associated with a function op and gets // cleaned when the latter is cleaned. } else { - processSimpleOp(op, la, deadVals, finalCleanUpList); + processSimpleOp(op, la, deadVals, finalRDVFinalCleanupList); } }); - cleanUpDeadVals(finalCleanUpList); + cleanUpDeadVals(finalRDVFinalCleanupList); } std::unique_ptr mlir::createRemoveDeadValuesPass() { From 398e8db5ea16b42ff9a52ed2da131c596bba767b Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Wed, 1 Jan 2025 19:24:47 +0000 Subject: [PATCH 08/23] Formatting --- mlir/lib/Transforms/RemoveDeadValues.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 8869ad48f7d2d..1b2d93af28262 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -228,7 +228,8 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// - A region branch terminator /// - Return-like static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, - DenseSet &deletionSet, RDVFinalCleanupList &cl) { + DenseSet &deletionSet, + RDVFinalCleanupList &cl) { if (!isMemoryEffectFree(op) || hasLive(op->getResults(), deletionSet, la)) return; @@ -639,7 +640,8 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, /// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, - DenseSet &deletionSet, RDVFinalCleanupList &cl) { + DenseSet &deletionSet, + RDVFinalCleanupList &cl) { unsigned numSuccessors = branchOp->getNumSuccessors(); // Do (1) @@ -742,7 +744,8 @@ void RemoveDeadValues::runOnOperation() { if (auto funcOp = dyn_cast(op)) { processFuncOp(funcOp, module, la, deadVals, finalRDVFinalCleanupList); } else if (auto regionBranchOp = dyn_cast(op)) { - processRegionBranchOp(regionBranchOp, la, deadVals, finalRDVFinalCleanupList); + processRegionBranchOp(regionBranchOp, la, deadVals, + finalRDVFinalCleanupList); } else if (auto branchOp = dyn_cast(op)) { processBranchOp(branchOp, la, deadVals, finalRDVFinalCleanupList); } else if (op->hasTrait<::mlir::OpTrait::IsTerminator>()) { From 30682b7f892a7885661b6e28ae1d4388941ba6e7 Mon Sep 17 00:00:00 2001 From: Renat Idrisov <4032256+parsifal-47@users.noreply.github.com> Date: Thu, 2 Jan 2025 09:10:38 -0800 Subject: [PATCH 09/23] Update mlir/lib/Transforms/RemoveDeadValues.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Andrzej Warzyński --- mlir/lib/Transforms/RemoveDeadValues.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 1b2d93af28262..4bf2b04832995 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -216,8 +216,8 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// Process a simple operation `op` using the liveness analysis `la`. /// If the operation has no memory effects and none of its results are live: -/// 1. Adding the operation to a list for future removal, and -/// 2. Marking all its results as non-live values +/// 1. Add the operation to a list for future removal, and +/// 2. Mark all its results as non-live values /// /// The operation `op` is assumed to be simple. /// A simple operation is one that is NOT: From 68606982be9fb4525bd7852c4b0ddb367b3b877a Mon Sep 17 00:00:00 2001 From: Renat Idrisov <4032256+parsifal-47@users.noreply.github.com> Date: Thu, 2 Jan 2025 09:10:47 -0800 Subject: [PATCH 10/23] Update mlir/lib/Transforms/RemoveDeadValues.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Andrzej Warzyński --- mlir/lib/Transforms/RemoveDeadValues.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 4bf2b04832995..6aa9dbeb7847a 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -219,8 +219,7 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// 1. Add the operation to a list for future removal, and /// 2. Mark all its results as non-live values /// -/// The operation `op` is assumed to be simple. -/// A simple operation is one that is NOT: +/// The operation `op` is assumed to be simple. A simple operation is one that is NOT: /// - Function-like /// - Call-like /// - A region branch operation From 11186970ba0f21d67daa4953cf3d31bee7dae384 Mon Sep 17 00:00:00 2001 From: Renat Idrisov <4032256+parsifal-47@users.noreply.github.com> Date: Thu, 2 Jan 2025 09:11:03 -0800 Subject: [PATCH 11/23] Update mlir/lib/Transforms/RemoveDeadValues.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Andrzej Warzyński --- mlir/lib/Transforms/RemoveDeadValues.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 6aa9dbeb7847a..2c55c2f40ff49 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -239,14 +239,17 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, /// Process a function-like operation `funcOp` using the liveness analysis `la` /// and the IR in `module`. If it is not public or external: -/// 1. Adding its non-live arguments to a list for future removal. -/// 2. Marking their corresponding operands in its callers for removal. -/// 3. Identifying and enqueueing unnecessary terminator operands +/// Process a function-like op `funcOp`, given the liveness information in `la` +/// and the IR in `module`. Here, processing means: +/// (1) Adding its non-live arguments to a list for future removal. +/// (2) Marking their corresponding operands in its callers for removal. +/// (3) Identifying and enqueueing unnecessary terminator operands /// (return values that are non-live across all callers) for removal. -/// 4. Enqueueing the non-live arguments themselves for removal. -/// 5. Collecting the uses of these return values in its callers for future +/// (4) Enqueueing the non-live arguments themselves for removal. +/// (5) Collecting the uses of these return values in its callers for future /// removal. /// 6. Marking all its results as non-live values. +/// iff it is not public or external. static void processFuncOp(FunctionOpInterface funcOp, Operation *module, RunLivenessAnalysis &la, DenseSet &deletionSet, RDVFinalCleanupList &cl) { From e9dec4021e2811c5637687f035442cd3a5cd30ad Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Thu, 2 Jan 2025 18:31:12 +0000 Subject: [PATCH 12/23] Addressing review feedback --- mlir/lib/Transforms/RemoveDeadValues.cpp | 70 ++++++++++++------------ 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 2c55c2f40ff49..0c1465600dbe6 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -112,10 +112,10 @@ struct RDVFinalCleanupList { /// Return true iff at least one value in `values` is live, given the liveness /// information in `la`. -static bool hasLive(ValueRange values, const DenseSet &deletionSet, +static bool hasLive(ValueRange values, const DenseSet &nonLiveSet, RunLivenessAnalysis &la) { for (Value value : values) { - if (deletionSet.contains(value)) + if (nonLiveSet.contains(value)) continue; const Liveness *liveness = la.getLiveness(value); @@ -128,12 +128,12 @@ static bool hasLive(ValueRange values, const DenseSet &deletionSet, /// Return a BitVector of size `values.size()` where its i-th bit is 1 iff the /// i-th value in `values` is live, given the liveness information in `la`. static BitVector markLives(ValueRange values, - const DenseSet &deletionSet, + const DenseSet &nonLiveSet, RunLivenessAnalysis &la) { BitVector lives(values.size(), true); for (auto [index, value] : llvm::enumerate(values)) { - if (deletionSet.contains(value)) { + if (nonLiveSet.contains(value)) { lives.reset(index); continue; } @@ -153,14 +153,14 @@ static BitVector markLives(ValueRange values, } /// Collects values marked as "non-live" in the provided range and inserts them -/// into the given set. A value is considered "non-live" if the corresponding +/// into the nonLiveSet. A value is considered "non-live" if the corresponding /// index in the `nonLive` bit vector is set. -static void collectNonLiveValues(DenseSet &deletionSet, ValueRange range, +static void collectNonLiveValues(DenseSet &nonLiveSet, ValueRange range, const BitVector &nonLive) { for (auto [index, result] : llvm::enumerate(range)) { if (!nonLive[index]) continue; - deletionSet.insert(result); + nonLiveSet.insert(result); } } @@ -227,13 +227,13 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// - A region branch terminator /// - Return-like static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, - DenseSet &deletionSet, + DenseSet &nonLiveSet, RDVFinalCleanupList &cl) { - if (!isMemoryEffectFree(op) || hasLive(op->getResults(), deletionSet, la)) + if (!isMemoryEffectFree(op) || hasLive(op->getResults(), nonLiveSet, la)) return; cl.operations.push_back(op); - collectNonLiveValues(deletionSet, op->getResults(), + collectNonLiveValues(nonLiveSet, op->getResults(), BitVector(op->getNumResults(), true)); } @@ -251,21 +251,21 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, /// 6. Marking all its results as non-live values. /// iff it is not public or external. static void processFuncOp(FunctionOpInterface funcOp, Operation *module, - RunLivenessAnalysis &la, DenseSet &deletionSet, + RunLivenessAnalysis &la, DenseSet &nonLiveSet, RDVFinalCleanupList &cl) { if (funcOp.isPublic() || funcOp.isExternal()) return; // Get the list of unnecessary (non-live) arguments in `nonLiveArgs`. SmallVector arguments(funcOp.getArguments()); - BitVector nonLiveArgs = markLives(arguments, deletionSet, la); + BitVector nonLiveArgs = markLives(arguments, nonLiveSet, la); nonLiveArgs = nonLiveArgs.flip(); // Do (1). for (auto [index, arg] : llvm::enumerate(arguments)) if (arg && nonLiveArgs[index]) { cl.values.push_back(arg); - deletionSet.insert(arg); + nonLiveSet.insert(arg); } // Do (2). @@ -312,11 +312,10 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, for (SymbolTable::SymbolUse use : uses) { Operation *callOp = use.getUser(); assert(isa(callOp) && "expected a call-like user"); - BitVector liveCallRets = markLives(callOp->getResults(), deletionSet, la); + BitVector liveCallRets = markLives(callOp->getResults(), nonLiveSet, la); nonLiveRets &= liveCallRets.flip(); } - // Do (4). // Note that in the absence of control flow ops forcing the control to go from // the entry (first) block to the other blocks, the control never reaches any // block other than the entry block, because every block has a terminator. @@ -325,6 +324,8 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, if (returnOp && returnOp->getNumOperands() == numReturns) cl.operands.push_back({returnOp, nonLiveRets}); } + + // Do (4). cl.functions.push_back({funcOp, nonLiveArgs, nonLiveRets}); // Do (5) and (6). @@ -332,7 +333,7 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, Operation *callOp = use.getUser(); assert(isa(callOp) && "expected a call-like user"); cl.results.push_back({callOp, nonLiveRets}); - collectNonLiveValues(deletionSet, callOp->getResults(), nonLiveRets); + collectNonLiveValues(nonLiveSet, callOp->getResults(), nonLiveRets); } } @@ -341,24 +342,24 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, /// /// Scenario 1: If the operation has no memory effects and none of its results /// are live: -/// 1. Enqueue all its uses for deletion. -/// 2. Enqueue the branch itself for deletion. +/// (1') Enqueue all its uses for deletion. +/// (2') Enqueue the branch itself for deletion. /// /// Scenario 2: Otherwise: -/// 1. Collect its unnecessary operands (operands forwarded to unnecessary +/// (1) Collect its unnecessary operands (operands forwarded to unnecessary /// results or arguments). -/// 2. Process each of its regions. -/// 3. Collect the uses of its unnecessary results (results forwarded from +/// (2) Process each of its regions. +/// (3) Collect the uses of its unnecessary results (results forwarded from /// unnecessary operands /// or terminator operands). -/// 4. Add these results to the deletion list. +/// (4) Add these results to the deletion list. /// /// Processing a region includes: -/// a. Collecting the uses of its unnecessary arguments (arguments forwarded +/// (a) Collecting the uses of its unnecessary arguments (arguments forwarded /// from unnecessary operands /// or terminator operands). -/// b. Collecting these unnecessary arguments. -/// c. Collecting its unnecessary terminator operands (terminator operands +/// (b) Collecting these unnecessary arguments. +/// (c) Collecting its unnecessary terminator operands (terminator operands /// forwarded to unnecessary results /// or arguments). /// @@ -367,18 +368,18 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, /// - To arguments and results (successor inputs). static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, RunLivenessAnalysis &la, - DenseSet &deletionSet, + DenseSet &nonLiveSet, RDVFinalCleanupList &cl) { // Mark live results of `regionBranchOp` in `liveResults`. auto markLiveResults = [&](BitVector &liveResults) { - liveResults = markLives(regionBranchOp->getResults(), deletionSet, la); + liveResults = markLives(regionBranchOp->getResults(), nonLiveSet, la); }; // Mark live arguments in the regions of `regionBranchOp` in `liveArgs`. auto markLiveArgs = [&](DenseMap &liveArgs) { for (Region ®ion : regionBranchOp->getRegions()) { SmallVector arguments(region.front().getArguments()); - BitVector regionLiveArgs = markLives(arguments, deletionSet, la); + BitVector regionLiveArgs = markLives(arguments, nonLiveSet, la); liveArgs[®ion] = regionLiveArgs; } }; @@ -566,8 +567,9 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, // case, a non-forwarded operand of `regionBranchOp` could be live/non-live. // It could never be live because of this op but its liveness could have been // attributed to something else. + // Do (1') and (2'). if (isMemoryEffectFree(regionBranchOp.getOperation()) && - !hasLive(regionBranchOp->getResults(), deletionSet, la)) { + !hasLive(regionBranchOp->getResults(), nonLiveSet, la)) { cl.operations.push_back(regionBranchOp.getOperation()); return; } @@ -616,7 +618,7 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, "implementing `RegionBranchOpInterface`"); BitVector argsToRemove = argsToKeep[®ion].flip(); cl.blocks.push_back({®ion.front(), argsToRemove}); - collectNonLiveValues(deletionSet, region.front().getArguments(), + collectNonLiveValues(nonLiveSet, region.front().getArguments(), argsToRemove); } @@ -629,7 +631,7 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, // Do (3) and (4). BitVector resultsToRemove = resultsToKeep.flip(); - collectNonLiveValues(deletionSet, regionBranchOp.getOperation()->getResults(), + collectNonLiveValues(nonLiveSet, regionBranchOp.getOperation()->getResults(), resultsToRemove); cl.results.push_back({regionBranchOp.getOperation(), resultsToRemove}); } @@ -642,7 +644,7 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, /// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, - DenseSet &deletionSet, + DenseSet &nonLiveSet, RDVFinalCleanupList &cl) { unsigned numSuccessors = branchOp->getNumSuccessors(); @@ -661,8 +663,8 @@ static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, // Do (3) BitVector successorNonLive = - markLives(operandValues, deletionSet, la).flip(); - collectNonLiveValues(deletionSet, successorBlock->getArguments(), + markLives(operandValues, nonLiveSet, la).flip(); + collectNonLiveValues(nonLiveSet, successorBlock->getArguments(), successorNonLive); cl.blocks.push_back({successorBlock, successorNonLive}); cl.successorOperands.push_back({branchOp, succIdx, successorNonLive}); From 58ca561f9b9833c7845039c708ed67d86871cccb Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Thu, 2 Jan 2025 18:31:32 +0000 Subject: [PATCH 13/23] Formatting --- mlir/lib/Transforms/RemoveDeadValues.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 0c1465600dbe6..91b09ad78c0c2 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -127,8 +127,7 @@ static bool hasLive(ValueRange values, const DenseSet &nonLiveSet, /// Return a BitVector of size `values.size()` where its i-th bit is 1 iff the /// i-th value in `values` is live, given the liveness information in `la`. -static BitVector markLives(ValueRange values, - const DenseSet &nonLiveSet, +static BitVector markLives(ValueRange values, const DenseSet &nonLiveSet, RunLivenessAnalysis &la) { BitVector lives(values.size(), true); From e2ca3dbce12c9482176354892bac9f1b9c428d28 Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Thu, 2 Jan 2025 18:38:32 +0000 Subject: [PATCH 14/23] Formatting --- mlir/lib/Transforms/RemoveDeadValues.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 91b09ad78c0c2..f591bd13380e1 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -218,7 +218,8 @@ static SmallVector operandsToOpOperands(OperandRange operands) { /// 1. Add the operation to a list for future removal, and /// 2. Mark all its results as non-live values /// -/// The operation `op` is assumed to be simple. A simple operation is one that is NOT: +/// The operation `op` is assumed to be simple. A simple operation is one that +/// is NOT: /// - Function-like /// - Call-like /// - A region branch operation From d60829f3959ebb06f7404678004c32c9c78cee12 Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Fri, 3 Jan 2025 21:13:24 +0000 Subject: [PATCH 15/23] Addressing review feedback --- mlir/lib/Transforms/RemoveDeadValues.cpp | 55 ++++++++++++------------ 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index f591bd13380e1..2d05748fe5dcc 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -239,17 +239,14 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, /// Process a function-like operation `funcOp` using the liveness analysis `la` /// and the IR in `module`. If it is not public or external: -/// Process a function-like op `funcOp`, given the liveness information in `la` -/// and the IR in `module`. Here, processing means: /// (1) Adding its non-live arguments to a list for future removal. /// (2) Marking their corresponding operands in its callers for removal. /// (3) Identifying and enqueueing unnecessary terminator operands /// (return values that are non-live across all callers) for removal. -/// (4) Enqueueing the non-live arguments themselves for removal. +/// (4) Enqueueing the non-live arguments and return values for removal. /// (5) Collecting the uses of these return values in its callers for future /// removal. -/// 6. Marking all its results as non-live values. -/// iff it is not public or external. +/// (6) Marking all its results as non-live values. static void processFuncOp(FunctionOpInterface funcOp, Operation *module, RunLivenessAnalysis &la, DenseSet &nonLiveSet, RDVFinalCleanupList &cl) { @@ -637,22 +634,22 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, } /// Steps to process a `BranchOpInterface` operation: -/// 1. Iterate through each successor block of the operation. -/// 2. For each successor block, gather all operands from all successors -/// along with their associated liveness analysis data. -/// 3. Identify and collect the dead operands from the branch operation -/// as well as their corresponding arguments. +/// Iterate through each successor block of the operation. +/// (1) For each successor block, gather all operands from all successors. +/// along with their associated liveness analysis data. +/// (2) Fetch their associated liveness analysis data. +/// (3) Identify and collect the dead operands from the branch operation +/// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, DenseSet &nonLiveSet, RDVFinalCleanupList &cl) { unsigned numSuccessors = branchOp->getNumSuccessors(); - // Do (1) for (unsigned succIdx = 0; succIdx < numSuccessors; ++succIdx) { Block *successorBlock = branchOp->getSuccessor(succIdx); - // Do (2) + // Do (1) SuccessorOperands successorOperands = branchOp.getSuccessorOperands(succIdx); SmallVector operandValues; @@ -661,46 +658,50 @@ static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, operandValues.push_back(successorOperands[operandIdx]); } - // Do (3) + // Do (2) BitVector successorNonLive = markLives(operandValues, nonLiveSet, la).flip(); collectNonLiveValues(nonLiveSet, successorBlock->getArguments(), successorNonLive); + + // Do (3) cl.blocks.push_back({successorBlock, successorNonLive}); cl.successorOperands.push_back({branchOp, succIdx, successorNonLive}); } } -static void cleanUpDeadVals(RDVFinalCleanupList &RDVFinalCleanupList) { +/// Removes dead values collected in RDVFinalCleanupList. +/// To be run once when all dead values have been collected. +static void cleanUpDeadVals(RDVFinalCleanupList &list) { // 1. Operations - for (auto &op : RDVFinalCleanupList.operations) { + for (auto &op : list.operations) { op->dropAllUses(); op->erase(); } // 2. Values - for (auto &v : RDVFinalCleanupList.values) { + for (auto &v : list.values) { v.dropAllUses(); } // 3. Functions - for (auto &f : RDVFinalCleanupList.functions) { + for (auto &f : list.functions) { f.funcOp.eraseArguments(f.nonLiveArgs); f.funcOp.eraseResults(f.nonLiveRets); } // 4. Operands - for (auto &o : RDVFinalCleanupList.operands) { + for (auto &o : list.operands) { o.op->eraseOperands(o.nonLive); } // 5. Results - for (auto &r : RDVFinalCleanupList.results) { + for (auto &r : list.results) { dropUsesAndEraseResults(r.op, r.nonLive); } // 6. Blocks - for (auto &b : RDVFinalCleanupList.blocks) { + for (auto &b : list.blocks) { // blocks that are accessed via multiple codepaths processed once if (b.b->getNumArguments() != b.nonLiveArgs.size()) continue; @@ -713,7 +714,7 @@ static void cleanUpDeadVals(RDVFinalCleanupList &RDVFinalCleanupList) { } // 7. Successor Operands - for (auto &op : RDVFinalCleanupList.successorOperands) { + for (auto &op : list.successorOperands) { SuccessorOperands successorOperands = op.branch.getSuccessorOperands(op.successorIndex); // blocks that are accessed via multiple codepaths processed once @@ -742,16 +743,16 @@ void RemoveDeadValues::runOnOperation() { // Maintains a list of Ops, values, branches, etc., slated for cleanup at the // end of this pass. - RDVFinalCleanupList finalRDVFinalCleanupList; + RDVFinalCleanupList finalCleanupList; module->walk([&](Operation *op) { if (auto funcOp = dyn_cast(op)) { - processFuncOp(funcOp, module, la, deadVals, finalRDVFinalCleanupList); + processFuncOp(funcOp, module, la, deadVals, finalCleanupList); } else if (auto regionBranchOp = dyn_cast(op)) { processRegionBranchOp(regionBranchOp, la, deadVals, - finalRDVFinalCleanupList); + finalCleanupList); } else if (auto branchOp = dyn_cast(op)) { - processBranchOp(branchOp, la, deadVals, finalRDVFinalCleanupList); + processBranchOp(branchOp, la, deadVals, finalCleanupList); } else if (op->hasTrait<::mlir::OpTrait::IsTerminator>()) { // Nothing to do here because this is a terminator op and it should be // honored with respect to its parent @@ -759,11 +760,11 @@ void RemoveDeadValues::runOnOperation() { // Nothing to do because this op is associated with a function op and gets // cleaned when the latter is cleaned. } else { - processSimpleOp(op, la, deadVals, finalRDVFinalCleanupList); + processSimpleOp(op, la, deadVals, finalCleanupList); } }); - cleanUpDeadVals(finalRDVFinalCleanupList); + cleanUpDeadVals(finalCleanupList); } std::unique_ptr mlir::createRemoveDeadValuesPass() { From ab30c3654676163a8fb0f4f9394d8ead6637fc97 Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Fri, 3 Jan 2025 21:21:34 +0000 Subject: [PATCH 16/23] Addressing review feedback --- mlir/lib/Transforms/RemoveDeadValues.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 2d05748fe5dcc..9f829679b67c8 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -636,9 +636,8 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, /// Steps to process a `BranchOpInterface` operation: /// Iterate through each successor block of the operation. /// (1) For each successor block, gather all operands from all successors. -/// along with their associated liveness analysis data. -/// (2) Fetch their associated liveness analysis data. -/// (3) Identify and collect the dead operands from the branch operation +/// (2) Fetch their associated liveness analysis data and collect for future removal. +/// (3) Identify and collect the dead operands from the successor block /// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, From 68b398a7c0a0170f5d02aadc97530b86b984c44e Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Fri, 3 Jan 2025 21:26:02 +0000 Subject: [PATCH 17/23] Formatting --- mlir/lib/Transforms/RemoveDeadValues.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 9f829679b67c8..26e9cbf6cd5b9 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -636,8 +636,8 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, /// Steps to process a `BranchOpInterface` operation: /// Iterate through each successor block of the operation. /// (1) For each successor block, gather all operands from all successors. -/// (2) Fetch their associated liveness analysis data and collect for future removal. -/// (3) Identify and collect the dead operands from the successor block +/// (2) Fetch their associated liveness analysis data and collect for future +/// removal. (3) Identify and collect the dead operands from the successor block /// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, From 3b8b16c568459c711c412738ce078261184d6574 Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Fri, 3 Jan 2025 21:55:25 +0000 Subject: [PATCH 18/23] Formatting --- mlir/lib/Transforms/RemoveDeadValues.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 26e9cbf6cd5b9..a9ddec971fb32 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -748,8 +748,7 @@ void RemoveDeadValues::runOnOperation() { if (auto funcOp = dyn_cast(op)) { processFuncOp(funcOp, module, la, deadVals, finalCleanupList); } else if (auto regionBranchOp = dyn_cast(op)) { - processRegionBranchOp(regionBranchOp, la, deadVals, - finalCleanupList); + processRegionBranchOp(regionBranchOp, la, deadVals, finalCleanupList); } else if (auto branchOp = dyn_cast(op)) { processBranchOp(branchOp, la, deadVals, finalCleanupList); } else if (op->hasTrait<::mlir::OpTrait::IsTerminator>()) { From 429ae509f200e32649d3821fb7203247df05df9e Mon Sep 17 00:00:00 2001 From: Renat Idrisov <4032256+parsifal-47@users.noreply.github.com> Date: Sun, 5 Jan 2025 08:49:46 -0800 Subject: [PATCH 19/23] Update mlir/lib/Transforms/RemoveDeadValues.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Andrzej Warzyński --- mlir/lib/Transforms/RemoveDeadValues.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index a9ddec971fb32..5ec1e9493e540 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -637,8 +637,9 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, /// Iterate through each successor block of the operation. /// (1) For each successor block, gather all operands from all successors. /// (2) Fetch their associated liveness analysis data and collect for future -/// removal. (3) Identify and collect the dead operands from the successor block -/// as well as their corresponding arguments. +/// removal. +/// (3) Identify and collect the dead operands from the successor block +/// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, DenseSet &nonLiveSet, From cf7b84fff090d5db72425fb742ebe2adfe81e507 Mon Sep 17 00:00:00 2001 From: Renat Idrisov <4032256+parsifal-47@users.noreply.github.com> Date: Sun, 5 Jan 2025 08:49:58 -0800 Subject: [PATCH 20/23] Update mlir/lib/Transforms/RemoveDeadValues.cpp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Andrzej Warzyński --- mlir/lib/Transforms/RemoveDeadValues.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 5ec1e9493e540..63fb5ea6cc9da 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -634,7 +634,7 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, } /// Steps to process a `BranchOpInterface` operation: -/// Iterate through each successor block of the operation. +/// Iterate through each successor block of `branchOp`. /// (1) For each successor block, gather all operands from all successors. /// (2) Fetch their associated liveness analysis data and collect for future /// removal. From 47a8fdaaf24fe763fdbd14402565ed7126959fc3 Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Sun, 5 Jan 2025 16:53:20 +0000 Subject: [PATCH 21/23] Addressing review feedback --- mlir/lib/Transforms/RemoveDeadValues.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 63fb5ea6cc9da..8a17e9b26c4da 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -705,6 +705,7 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) { // blocks that are accessed via multiple codepaths processed once if (b.b->getNumArguments() != b.nonLiveArgs.size()) continue; + // it iterates backwards because erase invalidates all successor indexes for (int i = b.nonLiveArgs.size() - 1; i >= 0; --i) { if (!b.nonLiveArgs[i]) continue; @@ -720,6 +721,7 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) { // blocks that are accessed via multiple codepaths processed once if (successorOperands.size() != op.nonLiveOperands.size()) continue; + // it iterates backwards because erase invalidates all successor indexes for (int i = successorOperands.size() - 1; i >= 0; --i) { if (!op.nonLiveOperands[i]) continue; From 7216aa1fe77dd44c87ba6f37ad0aac697e88608c Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Sun, 5 Jan 2025 16:57:27 +0000 Subject: [PATCH 22/23] Formatting --- mlir/lib/Transforms/RemoveDeadValues.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 8a17e9b26c4da..31d046911d4ea 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -637,7 +637,7 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, /// Iterate through each successor block of `branchOp`. /// (1) For each successor block, gather all operands from all successors. /// (2) Fetch their associated liveness analysis data and collect for future -/// removal. +/// removal. /// (3) Identify and collect the dead operands from the successor block /// as well as their corresponding arguments. From ec357811344d294f1c129abb894033862eb4fd02 Mon Sep 17 00:00:00 2001 From: Renat Idrisov Date: Wed, 8 Jan 2025 03:42:04 +0000 Subject: [PATCH 23/23] Addressing review feedback --- mlir/lib/Transforms/RemoveDeadValues.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp index 31d046911d4ea..3e7a0cca31c77 100644 --- a/mlir/lib/Transforms/RemoveDeadValues.cpp +++ b/mlir/lib/Transforms/RemoveDeadValues.cpp @@ -242,10 +242,10 @@ static void processSimpleOp(Operation *op, RunLivenessAnalysis &la, /// (1) Adding its non-live arguments to a list for future removal. /// (2) Marking their corresponding operands in its callers for removal. /// (3) Identifying and enqueueing unnecessary terminator operands -/// (return values that are non-live across all callers) for removal. +/// (return values that are non-live across all callers) for removal. /// (4) Enqueueing the non-live arguments and return values for removal. /// (5) Collecting the uses of these return values in its callers for future -/// removal. +/// removal. /// (6) Marking all its results as non-live values. static void processFuncOp(FunctionOpInterface funcOp, Operation *module, RunLivenessAnalysis &la, DenseSet &nonLiveSet, @@ -344,21 +344,21 @@ static void processFuncOp(FunctionOpInterface funcOp, Operation *module, /// /// Scenario 2: Otherwise: /// (1) Collect its unnecessary operands (operands forwarded to unnecessary -/// results or arguments). +/// results or arguments). /// (2) Process each of its regions. /// (3) Collect the uses of its unnecessary results (results forwarded from -/// unnecessary operands -/// or terminator operands). +/// unnecessary operands +/// or terminator operands). /// (4) Add these results to the deletion list. /// /// Processing a region includes: /// (a) Collecting the uses of its unnecessary arguments (arguments forwarded -/// from unnecessary operands -/// or terminator operands). +/// from unnecessary operands +/// or terminator operands). /// (b) Collecting these unnecessary arguments. /// (c) Collecting its unnecessary terminator operands (terminator operands -/// forwarded to unnecessary results -/// or arguments). +/// forwarded to unnecessary results +/// or arguments). /// /// Value Flow Note: In this operation, values flow as follows: /// - From operands and terminator operands (successor operands) @@ -637,9 +637,9 @@ static void processRegionBranchOp(RegionBranchOpInterface regionBranchOp, /// Iterate through each successor block of `branchOp`. /// (1) For each successor block, gather all operands from all successors. /// (2) Fetch their associated liveness analysis data and collect for future -/// removal. +/// removal. /// (3) Identify and collect the dead operands from the successor block -/// as well as their corresponding arguments. +/// as well as their corresponding arguments. static void processBranchOp(BranchOpInterface branchOp, RunLivenessAnalysis &la, DenseSet &nonLiveSet,