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

perf regression in pgm dispatch: 2.62->2.75 #12614

Open
pgkeller opened this issue Sep 12, 2024 · 3 comments
Open

perf regression in pgm dispatch: 2.62->2.75 #12614

pgkeller opened this issue Sep 12, 2024 · 3 comments
Assignees
Labels

Comments

@pgkeller
Copy link
Contributor

Commit 35d7055 dropped pgm dispatch perf from 2.62->2.75 secs w/ the following command:
build/test/tt_metal/perf_microbenchmark/dispatch/test_pgm_dispatch -w 5000 -s 256 -n -t

This issue is to track what about this changes caused the regression (using the other noc, riscv overhead) and if there is anything that can be done about it or if the change is even worthwhile (was hoped to show a gain by not programming the NOC each credit).

commit 1b421df (refs/bisect/good-1b421df0c467c5410b7756572fc710cdd45c9deb)
Author: Austin Ho [email protected]
Date: Tue Aug 13 18:24:20 2024 +0000

#11014: Use NOC 1 for dispatch upstream communication to support linking across subcmds
@tt-aho
Copy link
Contributor

tt-aho commented Sep 12, 2024

I didn't optimize the reprogramming when I flipped the noc, and just left it as is. Can try this to see if we gain back the lost perf. We were always reprogramming before this change as well though, so wouldn't have expected this to cause a drop.

Change is required to enable the mcast linking across subcmds for kernel bins so it's a needed change.

@pgkeller
Copy link
Contributor Author

I didn't optimize the reprogramming when I flipped the noc, and just left it as is. Can try this to see if we gain back the lost perf. We were always reprogramming before this change as well though, so wouldn't have expected this to cause a drop.

Change is required to enable the mcast linking across subcmds for kernel bins so it's a needed change.

oh, right forgot about that

@tt-aho
Copy link
Contributor

tt-aho commented Dec 4, 2024

@jbaumanTT found that we weren't actually linking mcast txns for different kernels even though on host we have sorted the data/cmds that target each core range. Ex trisc 0, 1, 2 mcast of binaries are linked, but trisc, ncrisc, and brisc mcasts are not linked even though they happen one after the other, so fixing this may resolve this perf drop that happened when we switched nocs to enable linking.

@jbaumanTT mentioned that it's on his todo to rewrite this part of the code, so we could wait if it's not a priority or I could put in a fix in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants