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 2022/03/22 21:21:37 UTC
[GitHub] [tvm] Lunderberg opened a new pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Lunderberg opened a new pull request #10710:
URL: https://github.com/apache/tvm/pull/10710
`CodeGenLLVM::CreatePrintf` is a debug function for injecting `printf` calls into the generated code. `CodeGenHexagon` inherits from `CodeGenLLVM`, but debug messages sent through `printf` calls do not make it back across the RPC server. Instead, the `FARF` preprocess macro provided from the Hexagon SDK should be used. This implementation of `CodeGenHexagon::CreatePrintf` generates the same `HAP_debug_v2` function call as would be generated by the `FARF` preprocessor macro, using the `ALWAYS` print level.
--
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
[GitHub] [tvm] csullivan merged pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
csullivan merged pull request #10710:
URL: https://github.com/apache/tvm/pull/10710
--
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
[GitHub] [tvm] kparzysz-quic commented on a change in pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on a change in pull request #10710:
URL: https://github.com/apache/tvm/pull/10710#discussion_r833210119
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -572,6 +574,43 @@ llvm::Value* CodeGenHexagon::CreateIntrinsic(const CallNode* op) {
return CodeGenLLVM::CreateIntrinsic(op);
}
+void CodeGenHexagon::CreatePrintf(const std::string& format,
+ const std::vector<llvm::Value*> format_args) {
+ // This function generates LLVM instructions to call HAP_debug_v2,
+ // as if the FARF macro in `HAP_farf.h` were called as
+ // FARF(ALWAYS, format, format_args[0], format_args[1], ...)
+ std::string func_name = "HAP_debug_v2";
+
+ llvm::Function* func = module_->getFunction(func_name);
+ if (func == nullptr) {
+ llvm::FunctionType* ftype = llvm::FunctionType::get(
+ t_void_, {t_int32_, t_char_->getPointerTo(), t_int32_, t_char_->getPointerTo()}, true);
+ func = llvm::Function::Create(ftype, llvm::Function::ExternalLinkage, func_name, module_.get());
+ }
+
+ llvm::Value* format_str = builder_->CreateGlobalStringPtr(format + "\0");
Review comment:
`format` already has `\0` at the end.
Also, you can pass the name as the second parameter (see llvm/include/llvm/IR/IRBuilder.h for reference), (same at line 600 below).
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -572,6 +574,43 @@ llvm::Value* CodeGenHexagon::CreateIntrinsic(const CallNode* op) {
return CodeGenLLVM::CreateIntrinsic(op);
}
+void CodeGenHexagon::CreatePrintf(const std::string& format,
+ const std::vector<llvm::Value*> format_args) {
Review comment:
Please use `llvm::ArrayRef` for non-mutable arrays (in LLVM-related code).
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -572,6 +574,43 @@ llvm::Value* CodeGenHexagon::CreateIntrinsic(const CallNode* op) {
return CodeGenLLVM::CreateIntrinsic(op);
}
+void CodeGenHexagon::CreatePrintf(const std::string& format,
+ const std::vector<llvm::Value*> format_args) {
+ // This function generates LLVM instructions to call HAP_debug_v2,
+ // as if the FARF macro in `HAP_farf.h` were called as
+ // FARF(ALWAYS, format, format_args[0], format_args[1], ...)
+ std::string func_name = "HAP_debug_v2";
+
+ llvm::Function* func = module_->getFunction(func_name);
+ if (func == nullptr) {
+ llvm::FunctionType* ftype = llvm::FunctionType::get(
+ t_void_, {t_int32_, t_char_->getPointerTo(), t_int32_, t_char_->getPointerTo()}, true);
+ func = llvm::Function::Create(ftype, llvm::Function::ExternalLinkage, func_name, module_.get());
+ }
+
+ llvm::Value* format_str = builder_->CreateGlobalStringPtr(format + "\0");
+ format_str->setName("printf_format_str");
+
+ // The value of FARF_ALWAYS_LEVEL, defined as HAP_LEVEL_HIGH
+ llvm::Value* level = ConstInt32(2);
+ level->setName("farf_always_level");
+
+ // There is no such filename for this print statement
+ llvm::Value* filename = builder_->CreateGlobalStringPtr("generated-LLVM-code");
+ filename->setName("dummy_filename");
+
+ // There is no such line number for this print statement
+ llvm::Value* line_number = ConstInt32(1);
+ line_number->setName("dummy_line_number");
+
+ std::vector<llvm::Value*> func_args = {level, filename, line_number, format_str};
+ for (auto arg : format_args) {
+ func_args.push_back(arg);
+ }
Review comment:
This loop can be replaced by `func_args.insert(func_args.end(), format_args.begin(), format_args.end());`.
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -572,6 +574,43 @@ llvm::Value* CodeGenHexagon::CreateIntrinsic(const CallNode* op) {
return CodeGenLLVM::CreateIntrinsic(op);
}
+void CodeGenHexagon::CreatePrintf(const std::string& format,
+ const std::vector<llvm::Value*> format_args) {
+ // This function generates LLVM instructions to call HAP_debug_v2,
+ // as if the FARF macro in `HAP_farf.h` were called as
+ // FARF(ALWAYS, format, format_args[0], format_args[1], ...)
+ std::string func_name = "HAP_debug_v2";
+
+ llvm::Function* func = module_->getFunction(func_name);
+ if (func == nullptr) {
+ llvm::FunctionType* ftype = llvm::FunctionType::get(
+ t_void_, {t_int32_, t_char_->getPointerTo(), t_int32_, t_char_->getPointerTo()}, true);
+ func = llvm::Function::Create(ftype, llvm::Function::ExternalLinkage, func_name, module_.get());
+ }
+
+ llvm::Value* format_str = builder_->CreateGlobalStringPtr(format + "\0");
+ format_str->setName("printf_format_str");
+
+ // The value of FARF_ALWAYS_LEVEL, defined as HAP_LEVEL_HIGH
+ llvm::Value* level = ConstInt32(2);
+ level->setName("farf_always_level");
Review comment:
In LLVM IR, integer immediates are always printed inline, so this name will never show anywhere (line 604 as well). There is no harm in having them though...
--
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
[GitHub] [tvm] Lunderberg commented on a change in pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on a change in pull request #10710:
URL: https://github.com/apache/tvm/pull/10710#discussion_r833610731
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -69,6 +69,9 @@ class CodeGenHexagon final : public CodeGenLLVM {
llvm::Module* GetModulePtr() const { return module_.get(); }
protected:
+ void CreatePrintf(const std::string& format,
+ const llvm::ArrayRef<llvm::Value*>& format_args) final;
Review comment:
Ah, whoops. Thank you for the catch, and sorry on that. I had been thinking it was a drop-in replacement for `std::vector` that owned its contents. I'm fine fixing it either here or in a different PR; either way works for me.
--
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
[GitHub] [tvm] csullivan commented on pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
csullivan commented on pull request #10710:
URL: https://github.com/apache/tvm/pull/10710#issuecomment-1076884322
Thanks @Lunderberg @kparzysz-quic !
--
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
[GitHub] [tvm] Lunderberg commented on a change in pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on a change in pull request #10710:
URL: https://github.com/apache/tvm/pull/10710#discussion_r833630813
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -69,6 +69,9 @@ class CodeGenHexagon final : public CodeGenLLVM {
llvm::Module* GetModulePtr() const { return module_.get(); }
protected:
+ void CreatePrintf(const std::string& format,
+ const llvm::ArrayRef<llvm::Value*>& format_args) final;
Review comment:
And updated.
--
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
[GitHub] [tvm] kparzysz-quic commented on a change in pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on a change in pull request #10710:
URL: https://github.com/apache/tvm/pull/10710#discussion_r833527843
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -69,6 +69,9 @@ class CodeGenHexagon final : public CodeGenLLVM {
llvm::Module* GetModulePtr() const { return module_.get(); }
protected:
+ void CreatePrintf(const std::string& format,
+ const llvm::ArrayRef<llvm::Value*>& format_args) final;
Review comment:
`llvm::ArrayRef` is meant to be passed by value, sorry, I should have been clearer about it (it's an array "view", like `std::string_view`). This can be fixed in a different PR though.
--
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
[GitHub] [tvm] Lunderberg commented on a change in pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on a change in pull request #10710:
URL: https://github.com/apache/tvm/pull/10710#discussion_r833359240
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -572,6 +574,43 @@ llvm::Value* CodeGenHexagon::CreateIntrinsic(const CallNode* op) {
return CodeGenLLVM::CreateIntrinsic(op);
}
+void CodeGenHexagon::CreatePrintf(const std::string& format,
+ const std::vector<llvm::Value*> format_args) {
+ // This function generates LLVM instructions to call HAP_debug_v2,
+ // as if the FARF macro in `HAP_farf.h` were called as
+ // FARF(ALWAYS, format, format_args[0], format_args[1], ...)
+ std::string func_name = "HAP_debug_v2";
+
+ llvm::Function* func = module_->getFunction(func_name);
+ if (func == nullptr) {
+ llvm::FunctionType* ftype = llvm::FunctionType::get(
+ t_void_, {t_int32_, t_char_->getPointerTo(), t_int32_, t_char_->getPointerTo()}, true);
+ func = llvm::Function::Create(ftype, llvm::Function::ExternalLinkage, func_name, module_.get());
+ }
+
+ llvm::Value* format_str = builder_->CreateGlobalStringPtr(format + "\0");
+ format_str->setName("printf_format_str");
+
+ // The value of FARF_ALWAYS_LEVEL, defined as HAP_LEVEL_HIGH
+ llvm::Value* level = ConstInt32(2);
+ level->setName("farf_always_level");
Review comment:
Thank you, and updated to remove these names. Agreed that there's no harm to it, so leaning in favor of avoiding extra line noise.
--
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
[GitHub] [tvm] Lunderberg commented on a change in pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on a change in pull request #10710:
URL: https://github.com/apache/tvm/pull/10710#discussion_r833352455
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -572,6 +574,43 @@ llvm::Value* CodeGenHexagon::CreateIntrinsic(const CallNode* op) {
return CodeGenLLVM::CreateIntrinsic(op);
}
+void CodeGenHexagon::CreatePrintf(const std::string& format,
+ const std::vector<llvm::Value*> format_args) {
+ // This function generates LLVM instructions to call HAP_debug_v2,
+ // as if the FARF macro in `HAP_farf.h` were called as
+ // FARF(ALWAYS, format, format_args[0], format_args[1], ...)
+ std::string func_name = "HAP_debug_v2";
+
+ llvm::Function* func = module_->getFunction(func_name);
+ if (func == nullptr) {
+ llvm::FunctionType* ftype = llvm::FunctionType::get(
+ t_void_, {t_int32_, t_char_->getPointerTo(), t_int32_, t_char_->getPointerTo()}, true);
+ func = llvm::Function::Create(ftype, llvm::Function::ExternalLinkage, func_name, module_.get());
+ }
+
+ llvm::Value* format_str = builder_->CreateGlobalStringPtr(format + "\0");
Review comment:
Thank you, and updated for both suggestions.
--
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
[GitHub] [tvm] Lunderberg commented on a change in pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on a change in pull request #10710:
URL: https://github.com/apache/tvm/pull/10710#discussion_r833366295
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -572,6 +574,43 @@ llvm::Value* CodeGenHexagon::CreateIntrinsic(const CallNode* op) {
return CodeGenLLVM::CreateIntrinsic(op);
}
+void CodeGenHexagon::CreatePrintf(const std::string& format,
+ const std::vector<llvm::Value*> format_args) {
+ // This function generates LLVM instructions to call HAP_debug_v2,
+ // as if the FARF macro in `HAP_farf.h` were called as
+ // FARF(ALWAYS, format, format_args[0], format_args[1], ...)
+ std::string func_name = "HAP_debug_v2";
+
+ llvm::Function* func = module_->getFunction(func_name);
+ if (func == nullptr) {
+ llvm::FunctionType* ftype = llvm::FunctionType::get(
+ t_void_, {t_int32_, t_char_->getPointerTo(), t_int32_, t_char_->getPointerTo()}, true);
+ func = llvm::Function::Create(ftype, llvm::Function::ExternalLinkage, func_name, module_.get());
+ }
+
+ llvm::Value* format_str = builder_->CreateGlobalStringPtr(format + "\0");
+ format_str->setName("printf_format_str");
+
+ // The value of FARF_ALWAYS_LEVEL, defined as HAP_LEVEL_HIGH
+ llvm::Value* level = ConstInt32(2);
+ level->setName("farf_always_level");
+
+ // There is no such filename for this print statement
+ llvm::Value* filename = builder_->CreateGlobalStringPtr("generated-LLVM-code");
+ filename->setName("dummy_filename");
+
+ // There is no such line number for this print statement
+ llvm::Value* line_number = ConstInt32(1);
+ line_number->setName("dummy_line_number");
+
+ std::vector<llvm::Value*> func_args = {level, filename, line_number, format_str};
+ for (auto arg : format_args) {
+ func_args.push_back(arg);
+ }
Review comment:
And updated, both here and the corresponding `CodeGenLLVM::CreatePrintf`
--
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
[GitHub] [tvm] Lunderberg commented on a change in pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on a change in pull request #10710:
URL: https://github.com/apache/tvm/pull/10710#discussion_r833367091
##########
File path: src/target/llvm/codegen_hexagon.cc
##########
@@ -572,6 +574,43 @@ llvm::Value* CodeGenHexagon::CreateIntrinsic(const CallNode* op) {
return CodeGenLLVM::CreateIntrinsic(op);
}
+void CodeGenHexagon::CreatePrintf(const std::string& format,
+ const std::vector<llvm::Value*> format_args) {
Review comment:
And updated, both here and in the corresponding `CodeGenLLVM::CreatePrintf`.
--
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