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 assume we are not passing the tvm_workspace_t struct to the radio
application, this example is accessing the next_alloc via some pointer
arithmetic on a nearby data that got linked in in the .data section ?
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:
[email protected]
