Trace Support #8572
Merged
Trace Support #8572
Conversation
This file contains 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
tt-asaigal
requested review from
eyonland,
arakhmati,
cfjchu,
xanderchin,
TT-BrianLiu,
tt-aho,
mywoodstock,
bbradelTT,
pgkeller,
abhullar-tt,
davorchap,
kmabeeTT,
tapspatel,
tt-rkim and
vtangTT
as code owners
tt-aho
approved these changes
May 16, 2024
bbradelTT
approved these changes
May 16, 2024
tapspatel
approved these changes
May 17, 2024
pgkeller
approved these changes
May 17, 2024
- Use noc_semaphore_inc(neg_val) + noc_async_atomic_barrier() as per Paul feedback, instead of ~unsafe direct set to ptr value
…tchQ MSB) - Host sets MSB of FetchQ entry on ExecBuf cmd to denote that prefetcher should stall and not fetch any more cmds since ExecBuf will read TraceBuffer data and write to CmdDataQ which will clobber subsequently fetched cmds. Remove previous "ugly hack" that was doing similiar thing. - On STALL_NEXT, barrier/wait for fetched cmd requesting a stall to return, and increase fence, before moving to STALLED state and early exit in fetch_q_get_cmds() when STALLED - PR Feedback and fix for NOT_STALLED setting - Add assert to make sure ExecBuf is comes with stall_flag=true, it's required now that ugly-hack is removed, otherwise hang. - Update test_prefetcher.cpp to set stall flag (FetchQ MSB) for ExecBuf, otherwise hang. Needed to change to make cmd_sizes uint32_t instead of uint16_t through the code since currently today FetchQ entry size dispatch_constants::prefetch_q_entry_type is uint32_t, to be able to carry though the MSB bit properly.
- Hang (assert with watcher) that STALL state wasn't seen when handling ExecBuf - Don't know if this is correct fix, but it seems to work...
Update device trace cmds to take in cq_id, remove multi-device apis Add tracing tests for metal Resnet50. TODO: Cleanup/reuse code Disable allocations after capturing trace Update trace apis to return/take in trace id. Make device own TraceBuffer mapping. Remove trace apis that correspond to allowing users to create Trace objects #8383: End any active traces during device close and assert tracing is not enabled for terminate cmd
- Add async safe ttnn and tt_lib trace APIs
- Single and multi-chip trace tests added to ttnn
post commit
- Resnet50 Async Trace tests added (after porting the model
over to async)
- Certain multichip tests with all-gather currently disabled
since they hang with trace
tt-asaigal
force-pushed
the
asaigal/ttnn_trace_rebased
branch
from
693956e
to
63e87ea
Compare
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.
fyi @tt-aho tracking all our trace commits here.