Lunderberg commented on code in PR #14532:
URL: https://github.com/apache/tvm/pull/14532#discussion_r1170211127
##########
src/tir/transforms/remove_no_op.cc:
##########
@@ -316,14 +323,19 @@ Pass RemoveNoOp() {
RemoveNoOpConfig config =
ctx->GetConfig<RemoveNoOpConfig>("tir.RemoveNoOp")
.value_or(AttrsWithDefaultValues<RemoveNoOpConfig>());
+
if (config->use_dataflow_analysis) {
- touch_pattern.emplace(f->body);
+ touch_pattern.emplace(f->body, config->max_simplification_steps);
}
arith::Analyzer analyzer;
+
analyzer.rewrite_simplify.SetMaximumRewriteSteps(config->max_simplification_steps);
- auto* n = f.CopyOnWrite();
- n->body = NoOpRemover::Apply(std::move(n->body), &analyzer,
std::move(touch_pattern), nullptr);
+ {
Review Comment:
They aren't necessary for correctness here, but were added present to limit
the scope of `write_ptr`. It doesn't have much impact in a function of this
size, especially with the return statement just afterward, but for longer
functions it can help to limit the scope of variables that are only needed for
a few following lines.
That said, the implementation no longer requires updating the signature of
`NoOpRemover::Apply`, so this change has become unrelated to the overall PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
