Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for watcher noc sanitization in noc_nonblocking_api.h #8334

Merged
merged 1 commit into from May 13, 2024

Conversation

tt-dma
Copy link
Contributor

@tt-dma tt-dma commented May 9, 2024

Had to do a bit of cleanup/rearranging in the headers to get around some circular dependencies + multiple definitions, let me know if anything looks off.

Passing CI: https://github.com/tenstorrent/tt-metal/actions/runs/9024330478

@tt-dma tt-dma requested a review from pgkeller May 9, 2024 23:25
@tt-dma tt-dma requested a review from aliuTT as a code owner May 9, 2024 23:25
@tt-dma tt-dma force-pushed the dma/6276_noc_nonblocking_debug branch from ccf6ca3 to 4a4d1d1 Compare May 9, 2024 23:48
@tt-dma
Copy link
Contributor Author

tt-dma commented May 10, 2024

Copy link
Contributor

@pgkeller pgkeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The noc checking was done at the dataflow layer because these noc layer are routines that may be shared w/ buda at some point. Please ping Milos on his plans before pushing this down to that layer. Otherwise, I'm ok w/ this.

@tt-dma
Copy link
Contributor Author

tt-dma commented May 10, 2024

Conclusion per Milos is to leave Metal-specific things out of noc_nonblocking_api.h, so remove that in this PR. New passing CI: https://github.com/tenstorrent/tt-metal/actions/runs/9037973534

I've added in the missing DEBUG_SANITIZE_* calls in front of all relevant noc_nonblocking_api.h calls, we'll need to be careful about these in the future - left a comment in that file mentioning this as well.

@tt-dma tt-dma requested a review from pgkeller May 10, 2024 21:36
Copy link
Contributor

@pgkeller pgkeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be sure to squash these commits so we don't have the changes in noc_* file histories

@tt-dma tt-dma force-pushed the dma/6276_noc_nonblocking_debug branch from 587f806 to 55e63bc Compare May 13, 2024 16:12
@tt-dma tt-dma merged commit 28df75f into main May 13, 2024
5 checks passed
@tt-dma tt-dma deleted the dma/6276_noc_nonblocking_debug branch May 13, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants