You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/05/17 09:51:50 UTC
[GitHub] [tvm] echuraev opened a new pull request #8054: [METAL] Fix codegen for inf and erf
echuraev opened a new pull request #8054:
URL: https://github.com/apache/tvm/pull/8054
Fixed Metal codegen with using `inf` constant. Constant `INFINITY` is
used now instead of `inf`.
Also, Metal doesn't have `erf` built-in function. So, we are using
`fast_erf` from tir. User will see warning message when we will
generate `fast_erf` instead of `erf`.
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] apeskov commented on a change in pull request #8054: [METAL] Fix codegen for inf and erf
Posted by GitBox <gi...@apache.org>.
apeskov commented on a change in pull request #8054:
URL: https://github.com/apache/tvm/pull/8054#discussion_r633559740
##########
File path: src/target/source/codegen_metal.h
##########
@@ -51,12 +51,13 @@ class CodeGenMetal final : public CodeGenC {
void PrintVecElemStore(const std::string& vec, DataType t, int i, const std::string& value) final;
// overload visitor
void VisitExpr_(const BroadcastNode* op, std::ostream& os) final; // NOLINT(*)
- // overload visitor
void VisitExpr_(const CallNode* op, std::ostream& os) final; // NOLINT(*)
+ void VisitExpr_(const FloatImmNode* op, std::ostream& os) final;
// reuse parent's function.
using CodeGenC::PrintType;
private:
+ friend void PrintConst(const FloatImmNode* op, std::ostream& os, CodeGenMetal* p);
Review comment:
Friend.. why not just class method?
##########
File path: src/target/source/intrin_rule_metal.cc
##########
@@ -90,6 +91,26 @@ TVM_REGISTER_OP("tir.cos").set_attr<FLowerIntrinsic>("metal.FLowerIntrinsic",
TVM_REGISTER_OP("tir.cosh")
.set_attr<FLowerIntrinsic>("metal.FLowerIntrinsic", DispatchPureExtern<Direct>);
+// There is no erf function in Metal. When erf is used, we use fast_erf instead
+static PrimExpr DispatchFastErf(const PrimExpr& e) {
+ LOG(WARNING) << " Metal doesn't have built-in erf function. fast_erf will be used instead.";
+ const CallNode* call = e.as<CallNode>();
+ ICHECK(call != nullptr);
+ ICHECK_EQ(call->args.size(), 1);
+ PrimExpr arg = call->args[0];
+ int bits = arg.dtype().bits();
+ bool isFloat = arg.dtype().is_float();
+ PrimExpr res;
+ if (isFloat && bits == 16)
+ res = topi::fast_erf_float_expr(arg, 16);
Review comment:
the same case `res = topi::fast_erf_float_expr(arg, bit);` , can be merged
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #8054: [METAL] Fix codegen for inf and erf
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #8054:
URL: https://github.com/apache/tvm/pull/8054#issuecomment-842631646
thanks @echuraev @apeskov
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on a change in pull request #8054: [METAL] Fix codegen for inf and erf
Posted by GitBox <gi...@apache.org>.
echuraev commented on a change in pull request #8054:
URL: https://github.com/apache/tvm/pull/8054#discussion_r633581589
##########
File path: src/target/source/codegen_metal.h
##########
@@ -51,12 +51,13 @@ class CodeGenMetal final : public CodeGenC {
void PrintVecElemStore(const std::string& vec, DataType t, int i, const std::string& value) final;
// overload visitor
void VisitExpr_(const BroadcastNode* op, std::ostream& os) final; // NOLINT(*)
- // overload visitor
void VisitExpr_(const CallNode* op, std::ostream& os) final; // NOLINT(*)
+ void VisitExpr_(const FloatImmNode* op, std::ostream& os) final;
// reuse parent's function.
using CodeGenC::PrintType;
private:
+ friend void PrintConst(const FloatImmNode* op, std::ostream& os, CodeGenMetal* p);
Review comment:
It was a copy-paste :) Thank you. Fixed.
##########
File path: src/target/source/intrin_rule_metal.cc
##########
@@ -90,6 +91,26 @@ TVM_REGISTER_OP("tir.cos").set_attr<FLowerIntrinsic>("metal.FLowerIntrinsic",
TVM_REGISTER_OP("tir.cosh")
.set_attr<FLowerIntrinsic>("metal.FLowerIntrinsic", DispatchPureExtern<Direct>);
+// There is no erf function in Metal. When erf is used, we use fast_erf instead
+static PrimExpr DispatchFastErf(const PrimExpr& e) {
+ LOG(WARNING) << " Metal doesn't have built-in erf function. fast_erf will be used instead.";
+ const CallNode* call = e.as<CallNode>();
+ ICHECK(call != nullptr);
+ ICHECK_EQ(call->args.size(), 1);
+ PrimExpr arg = call->args[0];
+ int bits = arg.dtype().bits();
+ bool isFloat = arg.dtype().is_float();
+ PrimExpr res;
+ if (isFloat && bits == 16)
+ res = topi::fast_erf_float_expr(arg, 16);
Review comment:
Done
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] apeskov commented on a change in pull request #8054: [METAL] Fix codegen for inf and erf
Posted by GitBox <gi...@apache.org>.
apeskov commented on a change in pull request #8054:
URL: https://github.com/apache/tvm/pull/8054#discussion_r633559740
##########
File path: src/target/source/codegen_metal.h
##########
@@ -51,12 +51,13 @@ class CodeGenMetal final : public CodeGenC {
void PrintVecElemStore(const std::string& vec, DataType t, int i, const std::string& value) final;
// overload visitor
void VisitExpr_(const BroadcastNode* op, std::ostream& os) final; // NOLINT(*)
- // overload visitor
void VisitExpr_(const CallNode* op, std::ostream& os) final; // NOLINT(*)
+ void VisitExpr_(const FloatImmNode* op, std::ostream& os) final;
// reuse parent's function.
using CodeGenC::PrintType;
private:
+ friend void PrintConst(const FloatImmNode* op, std::ostream& os, CodeGenMetal* p);
Review comment:
Friend.. why not just class method?
##########
File path: src/target/source/intrin_rule_metal.cc
##########
@@ -90,6 +91,26 @@ TVM_REGISTER_OP("tir.cos").set_attr<FLowerIntrinsic>("metal.FLowerIntrinsic",
TVM_REGISTER_OP("tir.cosh")
.set_attr<FLowerIntrinsic>("metal.FLowerIntrinsic", DispatchPureExtern<Direct>);
+// There is no erf function in Metal. When erf is used, we use fast_erf instead
+static PrimExpr DispatchFastErf(const PrimExpr& e) {
+ LOG(WARNING) << " Metal doesn't have built-in erf function. fast_erf will be used instead.";
+ const CallNode* call = e.as<CallNode>();
+ ICHECK(call != nullptr);
+ ICHECK_EQ(call->args.size(), 1);
+ PrimExpr arg = call->args[0];
+ int bits = arg.dtype().bits();
+ bool isFloat = arg.dtype().is_float();
+ PrimExpr res;
+ if (isFloat && bits == 16)
+ res = topi::fast_erf_float_expr(arg, 16);
Review comment:
the same case `res = topi::fast_erf_float_expr(arg, bit);` , can be merged
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on a change in pull request #8054: [METAL] Fix codegen for inf and erf
Posted by GitBox <gi...@apache.org>.
echuraev commented on a change in pull request #8054:
URL: https://github.com/apache/tvm/pull/8054#discussion_r633582444
##########
File path: src/target/source/intrin_rule_metal.cc
##########
@@ -90,6 +91,26 @@ TVM_REGISTER_OP("tir.cos").set_attr<FLowerIntrinsic>("metal.FLowerIntrinsic",
TVM_REGISTER_OP("tir.cosh")
.set_attr<FLowerIntrinsic>("metal.FLowerIntrinsic", DispatchPureExtern<Direct>);
+// There is no erf function in Metal. When erf is used, we use fast_erf instead
+static PrimExpr DispatchFastErf(const PrimExpr& e) {
+ LOG(WARNING) << " Metal doesn't have built-in erf function. fast_erf will be used instead.";
+ const CallNode* call = e.as<CallNode>();
+ ICHECK(call != nullptr);
+ ICHECK_EQ(call->args.size(), 1);
+ PrimExpr arg = call->args[0];
+ int bits = arg.dtype().bits();
+ bool isFloat = arg.dtype().is_float();
+ PrimExpr res;
+ if (isFloat && bits == 16)
+ res = topi::fast_erf_float_expr(arg, 16);
Review comment:
Done
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] echuraev commented on a change in pull request #8054: [METAL] Fix codegen for inf and erf
Posted by GitBox <gi...@apache.org>.
echuraev commented on a change in pull request #8054:
URL: https://github.com/apache/tvm/pull/8054#discussion_r633581589
##########
File path: src/target/source/codegen_metal.h
##########
@@ -51,12 +51,13 @@ class CodeGenMetal final : public CodeGenC {
void PrintVecElemStore(const std::string& vec, DataType t, int i, const std::string& value) final;
// overload visitor
void VisitExpr_(const BroadcastNode* op, std::ostream& os) final; // NOLINT(*)
- // overload visitor
void VisitExpr_(const CallNode* op, std::ostream& os) final; // NOLINT(*)
+ void VisitExpr_(const FloatImmNode* op, std::ostream& os) final;
// reuse parent's function.
using CodeGenC::PrintType;
private:
+ friend void PrintConst(const FloatImmNode* op, std::ostream& os, CodeGenMetal* p);
Review comment:
It was a copy-paste :) Thank you. Fixed.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi merged pull request #8054: [METAL] Fix codegen for inf and erf
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #8054:
URL: https://github.com/apache/tvm/pull/8054
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org