Skip to content

[Enhancement] Extension of initializing arbitrary bit integer in hcl.scalar from string - #431

Open
zzy82518996 wants to merge 7 commits into
cornell-zhang:tvmfrom
zzy82518996:string_scalar
Open

zzy82518996 wants to merge 7 commits into
cornell-zhang:tvmfrom
zzy82518996:string_scalar

Conversation

@zzy82518996

Copy link
Copy Markdown

For fixing issues

Add feature: enable initializing the arbitrary bit integer using hcl.scalar from string

Link to the tests: tests/test_scalar.py

Detailed Description : We extend the heteroCL IR by adding the CastStr expression, and use APInt class to convert string to ConstantInt class in codegen_llvm.

@hecmay
hecmay self-requested a review January 7, 2022 01:19

@hecmay hecmay left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zzy82518996

Can you please also pull back the latest changes in the master branch? Just want to make sure all the tests are passed before we merge it.

Comment thread tvm/src/pass/ir_mutator.cc
Comment thread tvm/src/codegen/llvm/llvm_common.cc
Comment thread tvm/src/codegen/llvm/llvm_module.cc Outdated
Comment on lines 156 to 176
void InitHB(Array<LoweredFunc> funcs, std::string target) {
InitializeLLVM();
CHECK(target.length() >= 11 && target.substr(0, 11) == "hammerblade");
std::ostringstream config;
config << "-mtriple=riscv32-unknown-unknown -mabi=ilp32f";
tm_ = GetLLVMTargetMachine(config.str());
std::unique_ptr<CodeGenLLVM> cg = CodeGenLLVM::Create(tm_);
ctx_ = std::make_shared<llvm::LLVMContext>();
std::unique_ptr<llvm::LLVMContext> ctx(new llvm::LLVMContext());
cg->Init(funcs[0]->name, tm_, ctx_.get(), false, false);
for (LoweredFunc f : funcs) {
cg->AddFunction(f);
}
cg->AddMainFunction(funcs[0]->name);
module_ = cg->Finish();
module_->addModuleFlag(llvm::Module::Warning, "tvm_target",
llvm::MDString::get(*ctx_, target));
target_ = target;
mptr_ = module_.get();
this->SaveToFile("hb_test.ll", "ll");
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for HB simulation in HCL right? I would suggest using another PR to introduce HB backend in HCL

Comment thread tvm/src/codegen/llvm/llvm_module.cc Outdated
Comment on lines +304 to +309
TVM_REGISTER_API("codegen.build_hammerblade")
.set_body([](TVMArgs args, TVMRetValue* rv) {
std::shared_ptr<LLVMModuleNode> n = std::make_shared<LLVMModuleNode>();
n->InitHB(args[0], args[1]);
*rv = runtime::Module(n);
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. HB target may as well be introduced separately.

Comment on lines +1587 to +1600
if (kernel_has_assert_[op->name]) {
llvm::BasicBlock* assert_true_kernel =
llvm::BasicBlock::Create(*ctx_, "assert_true_kernel", function_);
llvm::BasicBlock* assert_false_kernel =
llvm::BasicBlock::Create(*ctx_, "assert_false_kernel", function_);
llvm::Value* assert_flag =
builder_->CreateLoad(llvm::Type::getInt32Ty(*ctx_), assert_global_ptr_);
llvm::Value* cond = builder_->CreateICmpEQ(assert_flag, ConstInt32(1));
builder_->CreateCondBr(cond, assert_true_kernel, assert_false_kernel);
builder_->SetInsertPoint(assert_false_kernel);
AssertFreeVars();
builder_->CreateRet(ConstInt32(1));
builder_->SetInsertPoint(assert_true_kernel);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why inserting the kernel only when there is an assertion inside? can you please explain?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not modify this part, it is the same with what inside the master branch.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zzy82518996 can you run "git pull upstream master" to merge the latest changes in main branch? It seems that some changes are missing and the local CI/CD is not working as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants