Finite Lifetime for IO Tensors #51
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… have finite lifetime
…y are I/O buffers
Victor-Jung
force-pushed
the
pr/finite-lifetime-io
branch
from
0084047 to
dd1370c
Compare
Xeratec
changed the title
Xeratec
reviewed
Apr 17, 2025
Comment on lines
-50
to
+57
| uint32_t compute_num = snrt_cluster_compute_core_num(); | ||
| uint32_t compute_num = 1; //snrt_cluster_compute_core_num(); | ||
| int32_t ldI = compute_num * ${input_samples}; | ||
| int32_t batch_offset = ${seq_len} * ${input_samples}; | ||
|
|
||
| ${kernelName}(${data_in}, ${data_out}, ldI, batch_offset, batch_size, ${seq_len}, ${input_samples}); | ||
|
|
||
| // JUNGVI: This implementation is broken and has memory leak. | ||
| if (snrt_hartid() == 0){ | ||
| ${kernelName}(${data_in}, ${data_out}, ldI, batch_offset, batch_size, ${seq_len}, ${input_samples}); | ||
| } |
There was a problem hiding this comment.
Should this be part of this PR? How hard is it to fix it or did Run already fix it?
Comment on lines
305
to
+306
| # SCHEREMO: ConstantBuffers are assigned and allocated at compile time, Global Var Buffers are assigned at init time | ||
| if isinstance(ctxt.lookup(tensorMemoryConstraint.tensorName), ConstantBuffer) or ctxt.is_global( | ||
| tensorMemoryConstraint.tensorName): | ||
| if isinstance(ctxt.lookup(tensorMemoryConstraint.tensorName), ConstantBuffer): |
There was a problem hiding this comment.
Please adapt the comment according to you changes.
Comment on lines
+140
to
+141
| if hasattr(ctxt.lookup(buffer.name), "_alias"): | ||
| continue |
There was a problem hiding this comment.
Could it be helpful to mark aliased buffers with a special color or sth. else?
Comment on lines
-378
to
+383
| def setupModel(self, ctxt: NetworkContext, schedule: Schedule, layerBinding: OrderedDict[str, ONNXLayer], | ||
| def setupModel(self, ctxt: NetworkContext, schedule: Schedule, layerBinding: 'OrderedDict[str, ONNXLayer]', |
There was a problem hiding this comment.
Why this change? The ONNXLayer is imported above.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is based on this currently open PR, don't review it before PR#44 is merged.Added
VariableBufferis_output, andis_input, to indicate that aVariableBufferis an IO of the network. IO buffers have to be treated differently than normal buffers; while they live in the global scope, inputs need to be alive at the beginning of the computation (lifetime[0]=0), and outputs have to be alive at the end of the computation (lifetime[-1]=inf).Changed
_calculateLifetimesmethod of theMemoryScheduleris now giving a proper lifetime ofVariableBufferfrom the global scope.OrderedDictrepresenting the global context. This is necessary as other global buffers can depend on these arenas for their definition.Fixed
generateBufferAllocationCodefor PULP Deployer. Previously, the output was loaded in L3 at the beginning of the computation. This is completely unnecessary; breaks when the outputs don't have an infinite lifetime, and it is prone to error. Hence now the outputs are not loaded in L3.Softmax_fp32kernel for Snitch has a memory leak if used with 8 cores. I added a comment and constrained its execution to a single core._calculateLifetimes: The lifetime of aliased buffers is not correctly computed.PR Merge Checklist
develcommit and pointing todevel.CHANGELOG.mdfile has been updated.