Repository navigation
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/182016
Note: Links to docs will display an error until the docs builds have been completed. ❌ 2 New Failures, 3 Unrelated Failures, 10 Unclassified FailuresAs of commit 7a7fe4c with merge base e9ff322 ( NEW FAILURES - The following jobs have failed:
UNCLASSIFIED FAILURES - DrCI could not classify the following jobs because the workflow did not run on the merge base. The failures may be pre-existing on trunk or introduced by this PR:
FLAKY - The following jobs failed but were likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Skylion007
left a comment
There was a problem hiding this comment.
Oh will this fix the LTO issues I'm having I wonder...
|
Okay, this is why I hit a bunch of bugs when enabling IPO only on AARCH: #180186 |
| } | ||
|
|
||
| std::shared_ptr<RpcAgent> RpcAgent::currentRpcAgent_ = nullptr; | ||
| std::atomic<std::shared_ptr<RpcAgent>> RpcAgent::currentRpcAgent_{nullptr}; |
There was a problem hiding this comment.
A shared_ptr is implicitly atomic in ref count. Do you have the order swapped and mean a shared_ptr to
An atomic?
There was a problem hiding this comment.
I could be wrong, but my understanding is that the distinction here is between the std::shared_ptr control block and the std::shared_ptr object itself. The refcount operations are thread-safe, as you said, but concurrent access/modification of the same std::shared_ptr object still needs synchronisation, which is why the old code used:
std::shared_ptr<RpcAgent> RpcAgent::currentRpcAgent_ = nullptr;
...
std::atomic_load(¤tRpcAgent_)
std::atomic_compare_exchange_strong(¤tRpcAgent_, ...)
std::atomic_exchange(¤tRpcAgent_, ...)
GCC 15 now deprecates those APIs and suggests using std::atomic<std::shared_ptr<T>> instead:
/pytorch/torch/csrc/distributed/rpc/rpc_agent.cpp:259:26: error: 'std::shared_ptr<_Tp> std::atomic_load(const shared_ptr<_Tp>*) [with _Tp = torch::distributed::rpc::RpcAgent]' is deprecated: use 'std::atomic<std::shared_ptr<T>>' instead [-Werror=deprecated-declarations]
/pytorch/torch/csrc/distributed/rpc/rpc_agent.cpp:263:53: error: 'std::shared_ptr<_Tp> std::atomic_load(const shared_ptr<_Tp>*) [with _Tp = torch::distributed::rpc::RpcAgent]' is deprecated: use 'std::atomic<std::shared_ptr<T>>' instead [-Werror=deprecated-declarations]
/pytorch/torch/csrc/distributed/rpc/rpc_agent.cpp:277:40: error: 'bool std::atomic_compare_exchange_strong(shared_ptr<_Tp>*, shared_ptr<_Tp>*, shared_ptr<_Tp>) [with _Tp = torch::distributed::rpc::RpcAgent]' is deprecated: use 'std::atomic<std::shared_ptr<T>>' instead [-Werror=deprecated-declarations]
/pytorch/torch/csrc/distributed/rpc/rpc_agent.cpp:286:29: error: 'std::shared_ptr<_Tp> std::atomic_exchange(shared_ptr<_Tp>*, shared_ptr<_Tp>) [with _Tp = torch::distributed::rpc::RpcAgent]' is deprecated: use 'std::atomic<std::shared_ptr<T>>' instead [-Werror=deprecated-declarations]
cc1plus: all warnings being treated as errors
So I don’t think we want std::shared_ptr<std::atomic<...>> here, since the thing being atomically updated is the std::shared_ptr handle, not the RpcAgent object it points to.
There was a problem hiding this comment.
I'm not sure why this ifdef is needed, let's just deprecate all API and use modern one (but as part of separate PR)
| TARGET=final | ||
| GPU_IMAGE=arm64v8/almalinux:8 | ||
| DOCKER_GPU_BUILD_ARG=" --build-arg DEVTOOLSET_VERSION=13" | ||
| DOCKER_GPU_BUILD_ARG=" --build-arg DEVTOOLSET_VERSION=15" |
There was a problem hiding this comment.
Okay, so this is why is hit a bunch of bugs only AARCH when enabling LTO!
|
|
|
@aditew01 could you please rerun |
|
Below is a summary of the GCC 15 AArch64 issues encountered in this PR, along with their corresponding fixes: Compilation/build errors1. HeaderOnlyArrayRef empty vector constructionGCC 15 reports Failure message: Files changed:
2. RPC current agent atomic shared_ptr operationsGCC 15/libstdc++ deprecates the free Failure message: Files changed:
3. OperatorName namespace string insertion overlap
Failure message: Files changed:
Test failures4. SVE BF16 transpose strict aliasingThe previous Failure message: Files changed:
5. SVE PCH internal compiler errorsGCC reports an internal compiler error while compiling AArch64 SVE vector helpers through the Inductor precompiled-header path, matching GCC PR target/123457. The affected code is valid, but GCC 14/15 can ICE when those SVE intrinsics are compiled with Representative failure: Affected tests:
Files changed:
6.
|
|
@claude review |
|
Claude finished @aditew01's task in 2m 36s —— View job Code Review: PR #182016 — Update AArch64 toolchains to GCC15
Overall AssessmentThis is a well-structured PR that upgrades the AArch64 CI from GCC 13 to GCC 15 and fixes the resulting compilation issues. The fixes are correct and appropriately scoped. Below are detailed notes per change. 1. CI/Docker Changes (
|
|
@pytorchbot merge -i |
|
@pytorchbot revert -m "Breaks nightly whl builds" |
|
@pytorchbot revert -m "Breaks nightly whl builds" -c "nosignal" |
|
@pytorchbot successfully started a revert job. Check the current status here. |
Reverting PR 182016 failedReason: Command Details for Dev Infra teamRaised by workflow job |
This reverts commit d840765. Fixes: #188269 Pull Request resolved: #188408 Approved by: https://github.com/atalman, https://github.com/fadara01, https://github.com/malfet
`std::atomic_load`, `std::atomic_exchange`, and `std::atomic_compare_exchange_*` overloads for `std::shared_ptr<T>` are deprecated since C++20 ([ref](https://en.cppreference.com/cpp/memory/shared_ptr/atomic)). Newer GCC/libstdc++ warns on these deprecated overloads, and warning-as-error builds can turn this into a build failure, as encountered in pytorch#182016. This PR updates `RpcAgent::currentRpcAgent_` from a plain `std::shared_ptr<RpcAgent>` accessed through the deprecated free-function atomic API to `std::atomic<std::shared_ptr<RpcAgent>>` accessed through the modern member API when `__cpp_lib_atomic_shared_ptr` is available. The implementation keeps the legacy free-function path on older standard libraries that do not provide the C++20 `std::atomic<std::shared_ptr<T>>` specialization. cc. @aditew01 @malfet Pull Request resolved: pytorch#185633 Approved by: https://github.com/Skylion007, https://github.com/cyyever
Ahead of updating the AArch64 CI toolchains to GCC15 (pytorch#182016), this PR fixes issues that arise on the newer compiler version. ## Fixes included ### 1. HeaderOnlyArrayRef empty vector construction GCC 15 reports `-Werror=free-nonheap-object` from inlined `HeaderOnlyArrayRef<T>::vec()` while constructing an `std::vector<T>` from `Data` to `Data + Length`. For empty refs, `Data` may be null, and forming `Data + Length` still performs pointer arithmetic on that null pointer. The fix returns `{}` immediately for empty refs to avoid that. Failure message: ```text inlined from 'std::vector<T> c10::HeaderOnlyArrayRef<T>::vec() const [with T = long int]' at /pytorch/torch/headeronly/util/HeaderOnlyArrayRef.h:237:64, inlined from 'at::TensorIterator at::native::{anonymous}::_make_unfold_backward_iter_over_grad_out(at::Tensor&, const at::Tensor&, int64_t, int64_t, int64_t)' at /pytorch/aten/src/ATen/native/UnfoldBackward.h:52:65, inlined from 'void at::native::{anonymous}::unfold_backward_cpu_kernel(at::Tensor&, const at::Tensor&, int64_t, int64_t, int64_t)' at /pytorch/aten/src/ATen/native/cpu/UnfoldBackwardKernel.cpp:129:41: /usr/include/c++/15/bits/new_allocator.h:172:66: error: 'void operator delete(void*, std::size_t)' called on pointer '<unknown>' with nonzero offset [8, 9223372036854775800] [-Werror=free-nonheap-object] 172 | _GLIBCXX_OPERATOR_DELETE(_GLIBCXX_SIZED_DEALLOC(__p, __n)); | ^ cc1plus: some warnings being treated as errors ``` Files changed: - `torch/headeronly/util/HeaderOnlyArrayRef.h` ### 2. OperatorName namespace string insertion overlap `OperatorName::setNamespaceIfNotSet()` used `std::string::replace()` to copy from `name` back into an overlapping region of the same `name` buffer. GCC 15 inlines that path down to `memcpy` and reports `-Werror=restrict`. The fix uses `std::char_traits<char>::move()` for the overlapping move and `std::char_traits<char>::copy()` for the non-overlapping namespace prefix. Failure message: ```text inlined from 'constexpr std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::replace(size_type, size_type, const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&, size_type, size_type) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]' at /usr/include/c++/15/bits/basic_string.h:2434:29, inlined from 'bool c10::OperatorName::setNamespaceIfNotSet(const char*)' at /pytorch/aten/src/ATen/core/operator_name.h:44:19, inlined from 'bool c10::OperatorName::setNamespaceIfNotSet(const char*)' at /pytorch/aten/src/ATen/core/operator_name.h:38:8, inlined from 'virtual void OperatorNameTest_SetNamespaceIfNotSetWithExistingNamespace_Test::TestBody()' at /pytorch/aten/src/ATen/test/operator_name_test.cpp:17:58: /usr/include/c++/15/bits/char_traits.h:429:56: error: 'void* __builtin_memcpy(void*, const void*, long unsigned int)' accessing 28 bytes at offsets 11 and 0 overlaps 17 bytes at offset 11 [-Werror=restrict] 429 | return static_cast<char_type*>(__builtin_memcpy(__s1, __s2, __n)); | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~ cc1plus: some warnings being treated as errors ``` Files changed: - `aten/src/ATen/core/operator_name.h` ### 3. SVE PCH internal compiler errors GCC reports an internal compiler error while compiling AArch64 SVE vector helpers through the Inductor precompiled-header path, matching [GCC PR target/123457](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123457). The affected code is valid, but GCC 14/15 can ICE when those SVE intrinsics are compiled with `TORCH_INDUCTOR_PRECOMPILE_HEADERS` enabled. The workaround adds narrow `__attribute__((optimize("O0")))` guards around the specific helper functions that trigger the compiler bug for the affected GCC versions. Representative failure: ```text torch._inductor.exc.InductorError: CppCompileError: C++ compile error Output: during GIMPLE pass: ccp ... In member function 'at::vec::CPU_CAPABILITY::Vectorized<float> at::vec::CPU_CAPABILITY::Vectorized<float>::tanh() const': ... internal compiler error: Segmentation fault ``` Files changed: - `aten/src/ATen/cpu/vec/sve/vec_bfloat16.h` - `aten/src/ATen/cpu/vec/sve/vec_float.h` - `aten/src/ATen/cpu/vec/sve/vec_int.h` ### 4. SVE BF16 transpose tile aliasing The SVE BF16 transpose helper used `BFloat16` scratch tiles while also loading and storing those tiles through raw `uint16_t` SVE accesses. This keeps the existing tile representation and `uint16_t` helper path, but makes the scalar tile zero/copy operations access the raw `BFloat16::x` member. That avoids the mixed whole-object `BFloat16` / raw `uint16_t` access pattern that caused stale tile values with GCC15. Failure message: ```text ====================================================================== FAIL: test_transpose_copy (__main__.CPUReproTests) ---------------------------------------------------------------------- ... AssertionError: Tensor-likes are not close! Mismatched elements: 8 / 289 (2.8%) Greatest absolute difference: 2.0625 at index (16, 15) (up to 1e-05 allowed) Greatest relative difference: 1.0 at index (16, 8) (up to 0.016 allowed) ``` Files changed: - `aten/src/ATen/cpu/vec/sve/vec_bfloat16.h` Pull Request resolved: pytorch#187167 Approved by: https://github.com/aditew01, https://github.com/jansel
Based on pytorch#180850, this PR updates AArch64 CI toolchains to GCC15. The necessary GCC15 enablement fixes are part of pytorch#187167 and pytorch#185633. Pull Request resolved: pytorch#182016 Approved by: https://github.com/aditew01, https://github.com/Skylion007 Co-authored-by: Aditya Tewari <aditya.tewari@arm.com>
…#188408) This reverts commit d840765. Fixes: pytorch#188269 Pull Request resolved: pytorch#188408 Approved by: https://github.com/atalman, https://github.com/fadara01, https://github.com/malfet
…#188408) This reverts commit d840765. Fixes: pytorch#188269 Pull Request resolved: pytorch#188408 Approved by: https://github.com/atalman, https://github.com/fadara01, https://github.com/malfet
`std::atomic_load`, `std::atomic_exchange`, and `std::atomic_compare_exchange_*` overloads for `std::shared_ptr<T>` are deprecated since C++20 ([ref](https://en.cppreference.com/cpp/memory/shared_ptr/atomic)). Newer GCC/libstdc++ warns on these deprecated overloads, and warning-as-error builds can turn this into a build failure, as encountered in pytorch#182016. This PR updates `RpcAgent::currentRpcAgent_` from a plain `std::shared_ptr<RpcAgent>` accessed through the deprecated free-function atomic API to `std::atomic<std::shared_ptr<RpcAgent>>` accessed through the modern member API when `__cpp_lib_atomic_shared_ptr` is available. The implementation keeps the legacy free-function path on older standard libraries that do not provide the C++20 `std::atomic<std::shared_ptr<T>>` specialization. cc. @aditew01 @malfet Pull Request resolved: pytorch#185633 Approved by: https://github.com/Skylion007, https://github.com/cyyever
Ahead of updating the AArch64 CI toolchains to GCC15 (pytorch#182016), this PR fixes issues that arise on the newer compiler version. ## Fixes included ### 1. HeaderOnlyArrayRef empty vector construction GCC 15 reports `-Werror=free-nonheap-object` from inlined `HeaderOnlyArrayRef<T>::vec()` while constructing an `std::vector<T>` from `Data` to `Data + Length`. For empty refs, `Data` may be null, and forming `Data + Length` still performs pointer arithmetic on that null pointer. The fix returns `{}` immediately for empty refs to avoid that. Failure message: ```text inlined from 'std::vector<T> c10::HeaderOnlyArrayRef<T>::vec() const [with T = long int]' at /pytorch/torch/headeronly/util/HeaderOnlyArrayRef.h:237:64, inlined from 'at::TensorIterator at::native::{anonymous}::_make_unfold_backward_iter_over_grad_out(at::Tensor&, const at::Tensor&, int64_t, int64_t, int64_t)' at /pytorch/aten/src/ATen/native/UnfoldBackward.h:52:65, inlined from 'void at::native::{anonymous}::unfold_backward_cpu_kernel(at::Tensor&, const at::Tensor&, int64_t, int64_t, int64_t)' at /pytorch/aten/src/ATen/native/cpu/UnfoldBackwardKernel.cpp:129:41: /usr/include/c++/15/bits/new_allocator.h:172:66: error: 'void operator delete(void*, std::size_t)' called on pointer '<unknown>' with nonzero offset [8, 9223372036854775800] [-Werror=free-nonheap-object] 172 | _GLIBCXX_OPERATOR_DELETE(_GLIBCXX_SIZED_DEALLOC(__p, __n)); | ^ cc1plus: some warnings being treated as errors ``` Files changed: - `torch/headeronly/util/HeaderOnlyArrayRef.h` ### 2. OperatorName namespace string insertion overlap `OperatorName::setNamespaceIfNotSet()` used `std::string::replace()` to copy from `name` back into an overlapping region of the same `name` buffer. GCC 15 inlines that path down to `memcpy` and reports `-Werror=restrict`. The fix uses `std::char_traits<char>::move()` for the overlapping move and `std::char_traits<char>::copy()` for the non-overlapping namespace prefix. Failure message: ```text inlined from 'constexpr std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::replace(size_type, size_type, const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&, size_type, size_type) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]' at /usr/include/c++/15/bits/basic_string.h:2434:29, inlined from 'bool c10::OperatorName::setNamespaceIfNotSet(const char*)' at /pytorch/aten/src/ATen/core/operator_name.h:44:19, inlined from 'bool c10::OperatorName::setNamespaceIfNotSet(const char*)' at /pytorch/aten/src/ATen/core/operator_name.h:38:8, inlined from 'virtual void OperatorNameTest_SetNamespaceIfNotSetWithExistingNamespace_Test::TestBody()' at /pytorch/aten/src/ATen/test/operator_name_test.cpp:17:58: /usr/include/c++/15/bits/char_traits.h:429:56: error: 'void* __builtin_memcpy(void*, const void*, long unsigned int)' accessing 28 bytes at offsets 11 and 0 overlaps 17 bytes at offset 11 [-Werror=restrict] 429 | return static_cast<char_type*>(__builtin_memcpy(__s1, __s2, __n)); | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~ cc1plus: some warnings being treated as errors ``` Files changed: - `aten/src/ATen/core/operator_name.h` ### 3. SVE PCH internal compiler errors GCC reports an internal compiler error while compiling AArch64 SVE vector helpers through the Inductor precompiled-header path, matching [GCC PR target/123457](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123457). The affected code is valid, but GCC 14/15 can ICE when those SVE intrinsics are compiled with `TORCH_INDUCTOR_PRECOMPILE_HEADERS` enabled. The workaround adds narrow `__attribute__((optimize("O0")))` guards around the specific helper functions that trigger the compiler bug for the affected GCC versions. Representative failure: ```text torch._inductor.exc.InductorError: CppCompileError: C++ compile error Output: during GIMPLE pass: ccp ... In member function 'at::vec::CPU_CAPABILITY::Vectorized<float> at::vec::CPU_CAPABILITY::Vectorized<float>::tanh() const': ... internal compiler error: Segmentation fault ``` Files changed: - `aten/src/ATen/cpu/vec/sve/vec_bfloat16.h` - `aten/src/ATen/cpu/vec/sve/vec_float.h` - `aten/src/ATen/cpu/vec/sve/vec_int.h` ### 4. SVE BF16 transpose tile aliasing The SVE BF16 transpose helper used `BFloat16` scratch tiles while also loading and storing those tiles through raw `uint16_t` SVE accesses. This keeps the existing tile representation and `uint16_t` helper path, but makes the scalar tile zero/copy operations access the raw `BFloat16::x` member. That avoids the mixed whole-object `BFloat16` / raw `uint16_t` access pattern that caused stale tile values with GCC15. Failure message: ```text ====================================================================== FAIL: test_transpose_copy (__main__.CPUReproTests) ---------------------------------------------------------------------- ... AssertionError: Tensor-likes are not close! Mismatched elements: 8 / 289 (2.8%) Greatest absolute difference: 2.0625 at index (16, 15) (up to 1e-05 allowed) Greatest relative difference: 1.0 at index (16, 8) (up to 0.016 allowed) ``` Files changed: - `aten/src/ATen/cpu/vec/sve/vec_bfloat16.h` Pull Request resolved: pytorch#187167 Approved by: https://github.com/aditew01, https://github.com/jansel
Based on pytorch#180850, this PR updates AArch64 CI toolchains to GCC15. The necessary GCC15 enablement fixes are part of pytorch#187167 and pytorch#185633. Pull Request resolved: pytorch#182016 Approved by: https://github.com/aditew01, https://github.com/Skylion007 Co-authored-by: Aditya Tewari <aditya.tewari@arm.com>
…#188408) This reverts commit d840765. Fixes: pytorch#188269 Pull Request resolved: pytorch#188408 Approved by: https://github.com/atalman, https://github.com/fadara01, https://github.com/malfet
|
We hit a GCC13 compiler error unsigned_clamp #189953 |
Based on #180850, this PR updates AArch64 CI toolchains to GCC15.
The necessary GCC15 enablement fixes are part of #187167 and #185633.
cc @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @jerryzh168 @aditew01 @voznesenskym @penguinwu @EikanWang @Guobing-Chen @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @kadeng @muchulee8 @amjames @chauhang @aakhundov @coconutruben @jataylo @robert-hardwick