manupa-arm commented on a change in pull request #7785:
URL: https://github.com/apache/tvm/pull/7785#discussion_r619408979



##########
File path: src/runtime/crt/memory/stack_allocator.c
##########
@@ -16,17 +16,22 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-
 // LINT_C_FILE
-
 #include <tvm/runtime/crt/stack_allocator.h>
+#ifdef TVM_CRT_DEBUG
+#include <tvm/runtime/crt/logging.h>
+#endif
 
 void* StackMemoryManager_Allocate(tvm_workspace_t* tvm_runtime_workspace, 
int32_t nbytes) {
   uint32_t offset_bytes = (~nbytes + 1) & (TVM_RUNTIME_ALLOC_ALIGNMENT_BYTES - 
1);
   uint8_t* current_alloc = tvm_runtime_workspace->next_alloc;
   uint8_t* next_alloc = tvm_runtime_workspace->next_alloc + nbytes + 
offset_bytes;
   uint8_t* workspace_end = tvm_runtime_workspace->workspace + 
tvm_runtime_workspace->workspace_size;
-
+#ifdef TVM_CRT_DEBUG

Review comment:
       I get the negative consequences (therefore agree that we need a check) 
but not sure how realistic are they to be on by default. If we really want to 
have them by default -- @giuseros I'd suggest we'd need a seperate buffer to 
hold debug info not interleaved with the data buffer.
   
   We can always compile with -D STACK_ALLOCATOR_CHECK_ENABLED to unit test, is 
there a concern of not being able to unit test because of its not on by default 
?




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


Reply via email to