-
Notifications
You must be signed in to change notification settings - Fork 14
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 182c42462cd61515f60a11a46ee84f303ed073ee 2025-01-23 #1935
Conversation
3176c23
to
80bc95b
Compare
.github/Dockerfile.base
Outdated
@@ -39,7 +39,8 @@ RUN wget https://apt.llvm.org/llvm.sh && \ | |||
./llvm.sh 17 && \ | |||
apt install -y libc++-17-dev libc++abi-17-dev && \ | |||
ln -s /usr/bin/clang-17 /usr/bin/clang && \ | |||
ln -s /usr/bin/clang++-17 /usr/bin/clang++ | |||
ln -s /usr/bin/clang++-17 /usr/bin/clang++ && \ | |||
ln -s /usr/bin/clang-scan-deps-17 /usr/bin/clang-scan-deps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't love this change (is it also needed for dockers outside CI?), what is it fixing? Include path updates make sense and are typical for missing headers though, I like that part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm getting an error CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS-NOTFOUND
when building locally and in CI after the metal cmake change
This does fix it locally.
I also tried adding the cmake flag directly, but that didn't work for some reason.
I assume we get this error because something upstream is using scan deps. Probably the json dep update since it's been years between the updated and prev version.
ab65fac
to
70cf4a4
Compare
@@ -81,7 +81,7 @@ createOwnedTensor(std::shared_ptr<void> data, | |||
|
|||
return ::ttnn::Tensor( | |||
createStorage<OwnedStorage>(data.get(), numElements, dataType), | |||
::ttnn::Shape(small_vector_shape), utils::toTTNNDataType(dataType), | |||
::ttnn::SimpleShape(small_vector_shape), utils::toTTNNDataType(dataType), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last month explicit ShapeBase(std::span<const uint32_t>)
was added, we can now use ::ttnn::SimpleShape(shape)
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for noticing 🙏
I'll make the change.
…-DCMAKE_CXX_SCAN_FOR_MODULES=FALSE - Solves CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS-NOTFOUND error
Brings 31 tt-metal commits.
|
I paused tt-metal auto-uplifts so the 1am job didn't destroy this branch to let above tt-torch CI finish. Now that it finished, going to merge this and we'll kick off another auto uplift, can include the suggested Shape change on next one. |
This PR uplifts the third_party/tt-metal to the 182c42462cd61515f60a11a46ee84f303ed073ee