Repository navigation
fix(rccl): declare librocshmem.a as a byproduct of rocshmem_ext - #12331
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
|
@argus re-review |
![amd-argus[bot]](https://avatars.githubusercontent.com/in/4740020?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
412aa55 to
26f4b25
Compare
|
@argus re-review |
This comment has been minimized.
This comment has been minimized.
JIRA ID : LCOMPILER-2794 `./install.sh -t --rocshmem` fails to link four host test binaries. `init.cc` and `enqueue.cc` reach their `ENABLE_ROCSHMEM` arms, and nothing in `fakes/` defines the ten symbols those arms call. Each fake goes in the file named after the owning production TU; `rocshmem_fakes.cc` is new and conditional, since it includes `rocshmem.hpp`. Pre-existing, but unreachable under ninja until #12331 let `--rocshmem` start building. ## Test - `-t --ninja --rocshmem --address-sanitizer`: 3197/3197, was 3 link failures - `-t --ninja`: 4366/4366, unchanged
|
@Kapil-Shyam-Pawar @corey-derochie-amd gentle ping when you have a moment: this is approved by alex-breslow-amd, Argus is LGTM, and there are no open threads. It needs one more RCCL-Reviewers approval to merge. (The RCCL PR gate is re-running after a Ruby smoke infra failure.) Thanks! |
26f4b25 to
5c168a4
Compare
A clean --ninja --rocshmem or --rocshmem-gin build never starts. Ninja validates the whole graph before running any step, and two files named by absolute path have no edge producing them: librocshmem.a, linked by src/CMakeLists.txt and test/CMakeLists.txt, and rocshmem_config.h, which the GIN header copy takes as a DEPENDS. ExternalProject_Add declares no outputs unless asked, and the existing add_dependencies() calls contribute only order-only edges, which sequence the steps but do not say where the files come from -- and the load-time error fires before those steps could run anyway. The per-arch librocshmem_device_*.bc files need no entry: they are passed to the device compiles as command arguments, never as file-level dependencies. Verified from a tree with both build/ and ext/rocshmem removed, since ext/rocshmem lives in the source tree and a leftover copy hides the bug: --ninja --rocshmem 4010/4010 --ninja --rocshmem-gin 4051/4051 both previously stopped at [0/2].
5c168a4 to
13ca068
Compare
|
@corey-derochie-amd @Kapil-Shyam-Pawar gentle ping when you have a moment. I've rebased this onto current develop. That picks up #13706, which fixes the |
TheRock Submodule Bump ActivityNewest first
|
JIRA ID : LCOMPILER-2794
A clean
./install.sh --ninja --rocshmemnever starts:src/CMakeLists.txtandtest/CMakeLists.txtlink the archive by absolute path, so ninja needs an edge that produces it.ExternalProject_Adddeclares no outputs withoutBUILD_BYPRODUCTS, and the existingadd_dependencies(rccl rocshmem_ext)contributes only an order-only edge.The per-arch
librocshmem_device_*.bcfiles need no entry: they are passed as command arguments and sequenced by target dependencies onrocshmem_static.Test
Clean builds that previously stopped at
[0/2]now reach[4010/4010]and linklibrccl.so:./install.sh --ninja --rocshmem --amdgpu_targets gfx950--address-sanitizer