You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org> on 2025/01/20 09:19:26 UTC
[PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
PatrikPerssonInceptron opened a new pull request, #17598:
URL: https://github.com/apache/tvm/pull/17598
Updated FlopEstimator to include missing nodes that would otherwise cause tuning to fail for certain TIR functions
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org>.
PatrikPerssonInceptron commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1923479734
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
I was a bit unsure about this when I added the node. My original thought was that the attribute node would only serve to hold values for downstream passes such as codegen and would not be converted to executable code and therefore should not be in the flops estimate.
I updated the visitor to visit the body and value of AttrStmtNode.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org>.
PatrikPerssonInceptron commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1923479734
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
I was a bit unsure about this when I added the node. My original thought was that the attribute node would only serve to hold values for downstream passes such as codegen and would not be converted to executable code and therefore should not be in the flops estimate.
I updated the visitor to visit the body and value of AttrStmtNode, however I can undo this if it is not correct.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org>.
PatrikPerssonInceptron commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1923479734
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
I was a bit unsure about this when I added the node, but you are right. There could technically be a valid mops in the body. I updated the visitor to visit the body of AttrStmtNode.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org>.
PatrikPerssonInceptron commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1923479734
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
I was a bit unsure about this when I added the node, but you are right, there could technically be a valid mops in the body. I updated the visitor to visit the body of AttrStmtNode. In addition I added a visit to the value as well.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "wrongtest-intellif (via GitHub)" <gi...@apache.org>.
wrongtest-intellif merged PR #17598:
URL: https://github.com/apache/tvm/pull/17598
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org>.
PatrikPerssonInceptron commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1923606624
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
My original thought was that the attribute node would only serve to hold values for downstream passes such as codegen and would not be converted to executable code.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org>.
PatrikPerssonInceptron commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1923606624
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
My original thought was that the attribute node would only server to hold values for downstream passes such as codegen and would not be converted to executable code.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org>.
PatrikPerssonInceptron commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1923606624
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
My original thought was that the attribute node would only serve to hold values for downstream passes such as codegen and would not be converted to executable code.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "wrongtest-intellif (via GitHub)" <gi...@apache.org>.
wrongtest-intellif commented on PR #17598:
URL: https://github.com/apache/tvm/pull/17598#issuecomment-2617953805
@tvm-bot re-run
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org>.
PatrikPerssonInceptron commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1923479734
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
I was a bit unsure about this when I added the node, but you are right, there could technically be valid mops in the body. I updated the visitor to visit the body of AttrStmtNode. In addition I added a visit to the value as well.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "wrongtest-intellif (via GitHub)" <gi...@apache.org>.
wrongtest-intellif commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1922978258
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
Do we need `VisitStmt(op->body)` for attr annotation since there maybe valid mops under attr body?
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [TIR][FIX] update FlopEstimator to include missing nodes [tvm]
Posted by "PatrikPerssonInceptron (via GitHub)" <gi...@apache.org>.
PatrikPerssonInceptron commented on code in PR #17598:
URL: https://github.com/apache/tvm/pull/17598#discussion_r1923479734
##########
src/tir/analysis/estimate_flops.cc:
##########
@@ -138,6 +138,7 @@ class FlopEstimator : private ExprFunctor<TResult(const PrimExpr& n)>,
}
TResult VisitExpr_(const BufferLoadNode* op) override { return TResult(); }
+ TResult VisitStmt_(const AttrStmtNode* op) override { return TResult(); }
Review Comment:
I was a bit unsure about this when I added the node, but you are right. There could technically be a valid mops in the body. I updated the visitor to visit the body of AttrStmtNode. In addition I added a visit to the value as well.
--
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: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org