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

Uplift third_party/tt-metal to 2024-11-15 (42035c1711 + cherry-pick fix) #1295

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kmabeeTT
Copy link
Contributor

This gets us another week (Nov8 -> Nov15) of tt-metal. Using stable tt-metal 42035c1711 from tt-metal main plus a cherry pick fix for an assertion fix (not yet on tt-metal main, but expected to go in soon).

CI Runs:

tt-mlir (Passing): link
tt-forge-fe (Running) : link

Would merge if approved and if tt-forge-fe is passing.

runtime/lib/common/system_desc.cpp Outdated Show resolved Hide resolved
@kmabeeTT
Copy link
Contributor Author

kmabeeTT commented Nov 15, 2024

Will have to temporarily hold off merging this, since there is a regression in tt-forge-fe hit by CI.

https://github.com/tenstorrent/tt-forge-fe/actions/runs/11863453098/job/33064951475#step:11:25859

E       RuntimeError: TT_FATAL @ /__w/tt-forge-fe/tt-forge-fe/third_party/tt-mlir/third_party/tt-metal/src/tt-metal/ttnn/cpp/ttnn/operations/data_movement/pad/pad.cpp:41: rank == 4
E       info:
E       Tensor rank is not 4
E       backtrace:

Edit: Bisected to tt-metal commit here, tenstorrent/tt-metal@3112241, will open ticket.

Edit2: Metal ticket tenstorrent/tt-metal/issues/15167 solved and fix merged to tt-metal now.

nsmithtt added a commit that referenced this pull request Nov 18, 2024
If we include metal headers as SYSTEM headers, the compiler will ignore
warnings from these headers since they're explicitly marked out of tree.

For more context see:
#1295 (comment)
@kmabeeTT kmabeeTT force-pushed the kmabee/metal-uplift branch 2 times, most recently from 15c5c76 to 4149e71 Compare November 19, 2024 05:20
@kmabeeTT
Copy link
Contributor Author

I propose to merge this after Nick lands #1309 since it passes CI for tt-mlir and tt-forge-fe now when cherry-picking tt-metal fix for previous mentioned issue (added tt-mlir test to catch it yesterday in seperate PR) and because latest tt-metal exposes other failures. Link to passing CI:

tt-forge-fe: https://github.com/tenstorrent/tt-forge-fe/actions/runs/11906743486
tt-mlir: https://github.com/tenstorrent/tt-mlir/actions/runs/11906770146

@kmabeeTT kmabeeTT removed the request for review from tapspatel November 19, 2024 15:51
kmabeeTT pushed a commit that referenced this pull request Nov 19, 2024
If we include metal headers as SYSTEM headers, the compiler will ignore
warnings from these headers since they're explicitly marked out of tree.

For more context see:
#1295 (comment)
 - Cherry pick 2 metal fixes from Nov18 tt-metal main needed for tt-mlir/tt-forge-fe fails
 - Can't move to latest tt-metal, exposing other issues (sharding)
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.

3 participants