ANSHUMAN87 commented on a change in pull request #6147:
URL: https://github.com/apache/incubator-tvm/pull/6147#discussion_r461322467
##########
File path: src/printer/tir_text_printer.cc
##########
@@ -203,8 +185,36 @@ Doc TIRTextPrinter::PrintRange(const RangeNode* op) {
Doc TIRTextPrinter::PrintBuffer(const BufferNode* op) {
const Buffer& buffer = GetRef<Buffer>(op);
- CHECK_GT(memo_buf_.count(buffer), 0);
- return meta_->InMeta(buffer) ? meta_->GetMetaNode(buffer) :
memo_buf_[buffer];
+
+ if (meta_->InMeta(buffer)) {
+ return meta_->GetMetaNode(buffer);
+ } else if (memo_buf_.count(buffer)) {
+ return memo_buf_[buffer];
+ } else {
+ memo_buf_[buffer] = AllocBuf(buffer);
+ return BufferNode2Doc(op, memo_buf_[buffer]);
Review comment:
This is the case when a Buffer is encountered for first time.
I believe, the first case is always the declaration, so it needs to have all
the info related to the buffer.
Later on it will just use the unique_name always.
Below is a snapshot post my change:
```
attr [T_full_like: Buffer(T_full_like_1: handle, float32, [2, 3, 4], [])]
"realize_scope" = "";
realize(T_full_like, [0:2, 0:3, 0:4], True {
for (ax0.ax1.fused: int32, 0, 6) "parallel" {
for (ax2.inner: int32, 0, 4) "vectorized" {
T_full_like[floordiv(ax0.ax1.fused, 3), floormod(ax0.ax1.fused, 3),
ax2.inner] = 0f32
}
}
})
```
In above sample `T_full_like` is the buffer.
Please let me know, if my understanding is wrong here. TIA!
##########
File path: src/printer/tir_text_printer.cc
##########
@@ -203,8 +185,36 @@ Doc TIRTextPrinter::PrintRange(const RangeNode* op) {
Doc TIRTextPrinter::PrintBuffer(const BufferNode* op) {
const Buffer& buffer = GetRef<Buffer>(op);
- CHECK_GT(memo_buf_.count(buffer), 0);
- return meta_->InMeta(buffer) ? meta_->GetMetaNode(buffer) :
memo_buf_[buffer];
+
+ if (meta_->InMeta(buffer)) {
+ return meta_->GetMetaNode(buffer);
+ } else if (memo_buf_.count(buffer)) {
+ return memo_buf_[buffer];
+ } else {
+ memo_buf_[buffer] = AllocBuf(buffer);
+ return BufferNode2Doc(op, memo_buf_[buffer]);
Review comment:
Thanks! This also looks good to me. But i am not sure how to achieve it.
Can you please share some pointers, how to do that before encountering the
Buffer ?
Like in the example, the sequence of nodes are as below:
`AttrStmtNode(First encounter of Buffer) -> BufferRealizeNode -> ForNode
....`
Would you please help me, how to realize your point in this case ?
##########
File path: src/printer/tir_text_printer.cc
##########
@@ -203,8 +185,36 @@ Doc TIRTextPrinter::PrintRange(const RangeNode* op) {
Doc TIRTextPrinter::PrintBuffer(const BufferNode* op) {
const Buffer& buffer = GetRef<Buffer>(op);
- CHECK_GT(memo_buf_.count(buffer), 0);
- return meta_->InMeta(buffer) ? meta_->GetMetaNode(buffer) :
memo_buf_[buffer];
+
+ if (meta_->InMeta(buffer)) {
+ return meta_->GetMetaNode(buffer);
+ } else if (memo_buf_.count(buffer)) {
+ return memo_buf_[buffer];
+ } else {
+ memo_buf_[buffer] = AllocBuf(buffer);
+ return BufferNode2Doc(op, memo_buf_[buffer]);
Review comment:
Thanks! I can see your point clearly now.
But there are few points i like to be clear on, before adopting it.
1: This case is not `PrimFunc`. In this case it is `AttrStmtNode`. So there
will be scope specific buffers and blocks. Given that if we club all buffer
info at one place (like header as you suggested), it might be hard to correlate
for reader.
For example below:
```
attr [0] "extern_scope" = 0;
attr [new_size: Buffer(new_size_1: Pointer(int64), int64, [1], [])]
"realize_scope" = "global";
realize(new_size, [0:1], True {
attr [old_size: Buffer(old_size_1: Pointer(int64), int64, [1], [])]
"realize_scope" = "global";
realize(old_size, [0:1], True {
attr [skip: Buffer(skip_1: Pointer(int32), int32, [1], [])]
"realize_scope" = "global";
realize(skip, [0:1], True {
attr [infer_idx: Buffer(infer_idx_1: Pointer(int32), int32, [1],
[])] "realize_scope" = "global";
realize(infer_idx, [0:1], True {
attr [dst_idx: Buffer(dst_idx_1: Pointer(int32), int32, [1], [])]
"realize_scope" = "global";
realize(dst_idx, [0:1], True {
attr [src_idx: Buffer(src_idx_1: Pointer(int32), int32, [1],
[])] "realize_scope" = "global";
realize(src_idx, [0:1], True {
attr [data_shape: Buffer(data_shape_1: Pointer(int64), int64,
[3], [])] "realize_scope" = "global";
realize(data_shape, [0:3], True {
```
2: As this cant be handled in a generic way, there will be always a case
where we need to handle it. For now i can handle it for `AttrStmtNode`, later
on there can be more cases similar. So is that ok?
Please let me know your thoughts on above points. If required we can pull
others too :)
##########
File path: src/printer/tir_text_printer.cc
##########
@@ -203,8 +185,36 @@ Doc TIRTextPrinter::PrintRange(const RangeNode* op) {
Doc TIRTextPrinter::PrintBuffer(const BufferNode* op) {
const Buffer& buffer = GetRef<Buffer>(op);
- CHECK_GT(memo_buf_.count(buffer), 0);
- return meta_->InMeta(buffer) ? meta_->GetMetaNode(buffer) :
memo_buf_[buffer];
+
+ if (meta_->InMeta(buffer)) {
+ return meta_->GetMetaNode(buffer);
+ } else if (memo_buf_.count(buffer)) {
+ return memo_buf_[buffer];
+ } else {
+ memo_buf_[buffer] = AllocBuf(buffer);
+ return BufferNode2Doc(op, memo_buf_[buffer]);
Review comment:
Thanks a lot @spectrometerHBH for your valuable opinion. I am in total
agreement with you. Its just that we cover all the aspects of it :)
Let us have some more opinion on this point.
@tqchen , @junrushao1994, @FrozenGene, @MarisaKirisame , @zhiics : Would you
please share your thoughts on above points.
##########
File path: src/printer/tir_text_printer.cc
##########
@@ -203,8 +185,36 @@ Doc TIRTextPrinter::PrintRange(const RangeNode* op) {
Doc TIRTextPrinter::PrintBuffer(const BufferNode* op) {
const Buffer& buffer = GetRef<Buffer>(op);
- CHECK_GT(memo_buf_.count(buffer), 0);
- return meta_->InMeta(buffer) ? meta_->GetMetaNode(buffer) :
memo_buf_[buffer];
+
+ if (meta_->InMeta(buffer)) {
+ return meta_->GetMetaNode(buffer);
+ } else if (memo_buf_.count(buffer)) {
+ return memo_buf_[buffer];
+ } else {
+ memo_buf_[buffer] = AllocBuf(buffer);
+ return BufferNode2Doc(op, memo_buf_[buffer]);
Review comment:
Thanks @tqchen for sharing your opinion on this.
I believe if i am not mistaken, i will close this comment section as the
current changes takes care of logging Buffer details only once when it is
encountered for first time(assumed to be the declaration point) and use the
unique_id for later on references.
----------------------------------------------------------------
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]
