tkonolige commented on a change in pull request #7216:
URL: https://github.com/apache/tvm/pull/7216#discussion_r552228734
##########
File path: python/tvm/tir/buffer.py
##########
@@ -247,7 +247,9 @@ def decl_buffer(
shape_dtype = shape[0].dtype if hasattr(shape[0], "dtype") else "int32"
elem_offset = Var("%s_elem_offset" % name, shape_dtype)
if data is None:
- data = Var(name, PointerType(PrimType(dtype)), span)
+ # store bool as i8
+ storage_dtype = "int8" if dtype == "bool" else dtype
Review comment:
Why is there a special case for bool here?
##########
File path: src/target/source/codegen_cuda.cc
##########
@@ -581,7 +581,10 @@ void CodeGenCUDA::VisitStmt_(const AllocateNode* op) {
int32_t constant_size = op->constant_allocation_size();
ICHECK_GT(constant_size, 0) << "Can only handle constant size stack
allocation for now";
const VarNode* buffer = op->buffer_var.as<VarNode>();
- std::string scope = alloc_storage_scope_.at(buffer);
+ auto it = alloc_storage_scope_.find(buffer);
+ ICHECK(it != alloc_storage_scope_.end())
Review comment:
I think we could be a little more specific here. We need an `AttrStmt`
with a key of `storage_scope` right? So maybe say `"Buffer " << op->buffer_var
<< " is missing an AttrStmt with a \"storage_scope\" key"`.
##########
File path: src/tir/ir/stmt.cc
##########
@@ -274,9 +274,10 @@ TVM_STATIC_IR_FUNCTOR(ReprPrinter, vtable)
// Allocate
Allocate::Allocate(Var buffer_var, DataType dtype, Array<PrimExpr> extents,
PrimExpr condition,
Stmt body, Span span) {
- // TODO(tvm-team): Add invariant check to make sure
- // IsPointerPType(buffer_var->type_annotation, dtype)
- // once we fix the allocate tvm script printing.
+ ICHECK(IsPointerType(buffer_var->type_annotation, dtype))
+ << "Allocate: buffer_var expect to have the right pointer type
annotation"
+ << " annotation=" << buffer_var->type_annotation << ", dtype=" << dtype;
Review comment:
This should probably be a `CHECK` too if people are writing tvmscript.
##########
File path: src/tir/ir/stmt.cc
##########
@@ -274,9 +274,10 @@ TVM_STATIC_IR_FUNCTOR(ReprPrinter, vtable)
// Allocate
Allocate::Allocate(Var buffer_var, DataType dtype, Array<PrimExpr> extents,
PrimExpr condition,
Stmt body, Span span) {
- // TODO(tvm-team): Add invariant check to make sure
- // IsPointerPType(buffer_var->type_annotation, dtype)
- // once we fix the allocate tvm script printing.
+ ICHECK(IsPointerType(buffer_var->type_annotation, dtype))
+ << "Allocate: buffer_var expect to have the right pointer type
annotation"
+ << " annotation=" << buffer_var->type_annotation << ", dtype=" << dtype;
Review comment:
This might be a better error message:
```suggestion
<< "The allocated data type (" << dtype << ") does not match the type
annotation of the buffer " << buffer_var << " (" << buffer_var->type_annotation
<< "). The data type should be an element of the pointer type."
```
It does seem like we could infer the type from the buffer_var 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.
For queries about this service, please contact Infrastructure at:
[email protected]
