[GitHub] [tvm] kparzysz-quic commented on a change in pull request #10710: [Hexagon][Codegen] Implement CodeGenHexagon::CreatePrintf

[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