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 2020/07/29 08:00:11 UTC

[GitHub] [incubator-tvm] spectrometerHBH commented on a change in pull request #6147: [TIR][Logger]Buffer logger assert removed

spectrometerHBH commented on a change in pull request #6147:
URL: https://github.com/apache/incubator-tvm/pull/6147#discussion_r461320483



##########
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:
       I think we should return memo_buf_[buffer] here? Not very sure.

##########
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 think we can put the declaration in the headers, rather in the position where we first encounter it, which is buffers={...}. Then in your example, we can just print
   ```
   attr [T_full_like] "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
       }
     }
   })
   
   ```

##########
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 think we can put the declaration in the headers, rather in the position where we first encounter it, which is buffers={...}. Then in your example, we can just print
   ```
   buffers = {T_full_like: Buffer(T_full_like_1: handle, float32, [2, 3, 4], [])}
   
   ...
   
   attr [T_full_like] "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
       }
     }
   })
   
   ```

##########
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:
       We can print the buffer declaration after we print the body of PrimFunc, and with the help of Doc we can put the printed buffer declaration before the printed body.
   
   ```
   bodyDoc = Print(func.body)
   bufferDoc = ... // scan memo_buf_
   FinalDoc = bufferDoc << bodyDoc
   ```

##########
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'm not very sure, but I think Buffer is actually a global concept. Meanwhile, I think the way we take now is general, no matter what new nodes come in in the future.




----------------------------------------------------------------
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:
users@infra.apache.org