[CLR][Windows] Make Comgr DLL name configurable via CMake by lamb-j · Pull Request #4211 · ROCm/rocm-systems

lamb-j · 2026-03-19T02:09:29Z

Motivation

CLR's Comgr::LoadLib() constructs the Comgr DLL filename at runtime by concatenating HIP_MAJOR_VERSION and HIP_MINOR_VERSION compile defs (e.g., amd_comgr0702.dll). This couples CLR to a specific HIP-derived naming scheme and forces the Comgr build to produce a matching versioned name.

Move the DLL name from a runtime computation to a build-time CMake configuration so that build systems (TheRock, Adrenaline Driver, etc.) can pick whatever DLL name is appropriate for their packaging.

Technical Details

Replace the runtime is_versioned parameter in Comgr::LoadLib() with a CMake cache variable COMGR_DLL_NAME that's compiled into CLR as the COMGR_DLL_NAME preprocessor macro.

Default: amd_comgr.dll
Override: -DCOMGR_DLL_NAME=custom_name.dll

Aligns with ROCm/llvm-project#1278 (merged), which adds the same COMGR_DLL_NAME cache variable to the Comgr build itself. With both changes in place, the Comgr OUTPUT_NAME and the CLR load name are configured by a single source of truth.

Use cases:

TheRock: uses the default amd_comgr.dll, getting any version info from the DLL's version resource or via Comgr version APIs.
Adrenaline Driver: can override to amd_comgr_3.dll, amd_comgr_opencl.dll, etc.

JIRA ID

N/A

Test Plan

CI builds and tests against TheRock with this change paired with a TheRock-side update that passes -DCOMGR_DLL_NAME=... to both CLR builds.

Test Result

To be filled in once CI completes.

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

…ning Replace the runtime is_versioned parameter in Comgr::LoadLib with a CMake cache variable COMGR_DLL_NAME. This simplifies the DLL loading logic and allows the DLL name to be configured at build time rather than computed at runtime based on HIP version numbers. Co-Authored-By: Claude <noreply@anthropic.com>

Add a patch for rocm-systems that replaces the runtime is_versioned logic in Comgr::LoadLib with a CMake-configured COMGR_DLL_NAME compile definition. This allows TheRock to control the Comgr DLL name at build time (defaulting to amd_comgr.dll) without waiting for the upstream CLR change (rocm-systems PR #4211) to merge. Upstream PR: ROCm/rocm-systems#4211 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

lamb-j · 2026-04-14T15:11:08Z

@ROCm/lrt-build-infra this one is ready for review

## Summary Replace the deferred `set_target_properties` hack in the Comgr pre-hook with the `COMGR_DLL_NAME` cache variable introduced by ROCm/llvm-project#1278 (merged). Also pass `COMGR_DLL_NAME` to CLR builds so that ROCm/rocm-systems#4211 (pending) is picked up automatically when the submodule bumps. This is a transitional change — no patches, no submodule bumps. Both mechanisms keep the versioned DLL name (`amd_comgr0702.dll`) so existing CLR runtime loading logic continues to work. **Supersedes #4056** (same goal, different approach — pre-hook instead of patches). ### Transition matrix (Windows) | Comgr | CLR | Comgr DLL name | CLR loads | Match? | |-------|-----|----------------|-----------|--------| | Old (current) | Old (current) | Versioned via deferred fallback | Versioned via runtime logic | ✓ | | New (post llvm-project#1278) | Old | Versioned via `COMGR_DLL_NAME` cache var | Versioned via runtime logic | ✓ | | New | New (post rocm-systems#4211) | Versioned via `COMGR_DLL_NAME` cache var | Versioned via `COMGR_DLL_NAME` compile def | ✓ | ### Follow-up cleanup (separate PR, after both submodules bump) - Remove `COMGR_DLL_NAME` from pre-hook and CLR `CMAKE_ARGS` (both default to `amd_comgr.dll`) - Remove deferred `set_target_properties` fallback - Optionally remove `THEROCK_HIP_MAJOR/MINOR_VERSION` from amd-comgr `CMAKE_ARGS` ## Test plan - [ ] Linux CI passes (all new logic is `if(WIN32)` guarded — no-op on Linux) - [ ] Windows CI produces correctly named Comgr DLL - [ ] CLR loads Comgr successfully at runtime on Windows Co-authored-by: Claude <noreply@anthropic.com>

…k-comgr-name

Bumps from 2026-04-07 commit (be15507) to 2026-04-20 commit (a3e6827) to pick up TheRock #4535, which adds COMGR_DLL_NAME wiring needed by this PR's CLR changes. Co-Authored-By: Claude <noreply@anthropic.com>

## Summary Replace the deferred `set_target_properties` hack in the Comgr pre-hook with the `COMGR_DLL_NAME` cache variable introduced by ROCm/llvm-project#1278 (merged). Also pass `COMGR_DLL_NAME` to CLR builds so that ROCm/rocm-systems#4211 (pending) is picked up automatically when the submodule bumps. This is a transitional change — no patches, no submodule bumps. Both mechanisms keep the versioned DLL name (`amd_comgr0702.dll`) so existing CLR runtime loading logic continues to work. **Supersedes #4056** (same goal, different approach — pre-hook instead of patches). ### Transition matrix (Windows) | Comgr | CLR | Comgr DLL name | CLR loads | Match? | |-------|-----|----------------|-----------|--------| | Old (current) | Old (current) | Versioned via deferred fallback | Versioned via runtime logic | ✓ | | New (post llvm-project#1278) | Old | Versioned via `COMGR_DLL_NAME` cache var | Versioned via runtime logic | ✓ | | New | New (post rocm-systems#4211) | Versioned via `COMGR_DLL_NAME` cache var | Versioned via `COMGR_DLL_NAME` compile def | ✓ | ### Follow-up cleanup (separate PR, after both submodules bump) - Remove `COMGR_DLL_NAME` from pre-hook and CLR `CMAKE_ARGS` (both default to `amd_comgr.dll`) - Remove deferred `set_target_properties` fallback - Optionally remove `THEROCK_HIP_MAJOR/MINOR_VERSION` from amd-comgr `CMAKE_ARGS` ## Test plan - [ ] Linux CI passes (all new logic is `if(WIN32)` guarded — no-op on Linux) - [ ] Windows CI produces correctly named Comgr DLL - [ ] CLR loads Comgr successfully at runtime on Windows Co-authored-by: Claude <noreply@anthropic.com>

…nges Follow-up cleanup to #4535. With both ROCm/llvm-project#1278 (Comgr) and ROCm/rocm-systems#4211 (CLR) included in TheRock's submodule pins, the versioned-name bridging code is no longer needed. Both Comgr and CLR now default to amd_comgr.dll on Windows. Changes: - compiler/pre_hook_amd-comgr.cmake: drop the COMGR_DLL_NAME cache var and deferred set_target_properties fallback. - core/CMakeLists.txt: drop the _comgr_dll_name computation and the -DCOMGR_DLL_NAME=... args passed to hip-clr and ocl-clr. - compiler/CMakeLists.txt: drop -DTHEROCK_HIP_MAJOR_VERSION and -DTHEROCK_HIP_MINOR_VERSION from amd-comgr CMAKE_ARGS; new Comgr no longer reads them. Co-Authored-By: Claude <noreply@anthropic.com>

Add a patch for rocm-systems that replaces the runtime is_versioned logic in Comgr::LoadLib with a CMake-configured COMGR_DLL_NAME compile definition. This allows TheRock to use the new amd_comgr.dll name without waiting for the upstream CLR change to be picked up via submodule bump. Upstream PR: ROCm/rocm-systems#4211 Remove this patch once the rocm-systems submodule pin includes the upstream PR. Co-Authored-By: Claude <noreply@anthropic.com>

The CLR code changes in this PR don't actually require touching the TheRock CI workflow files. The earlier ref bumps were only added to test the CLR changes against a TheRock that included the matching COMGR_DLL_NAME wiring. Resetting the 7 therock-*.yml files to develop's current state. Once develop's TheRock ref naturally advances to include that wiring (TheRock PR #4535), this PR's CLR changes will pair correctly with no workflow edits needed here. Co-Authored-By: Claude <noreply@anthropic.com>

These files drifted from develop in earlier merges and have nothing to do with the CLR change in this PR. Co-Authored-By: Claude <noreply@anthropic.com>

…k-comgr-name-no-workflow

lamb-j · 2026-04-28T17:26:57Z

@ROCm/rocm-systems-reviewers I think the failing checks are unreleated. Can we land this one? Thanks!

…nges Follow-up cleanup to #4535. With both ROCm/llvm-project#1278 (Comgr) and ROCm/rocm-systems#4211 (CLR) included in TheRock's submodule pins, the versioned-name bridging code is no longer needed. Both Comgr and CLR now default to amd_comgr.dll on Windows. Changes: - compiler/pre_hook_amd-comgr.cmake: drop the COMGR_DLL_NAME cache var and deferred set_target_properties fallback. - core/CMakeLists.txt: drop the _comgr_dll_name computation and the -DCOMGR_DLL_NAME=... args passed to hip-clr and ocl-clr. - compiler/CMakeLists.txt: drop -DTHEROCK_HIP_MAJOR_VERSION and -DTHEROCK_HIP_MINOR_VERSION from amd-comgr CMAKE_ARGS; new Comgr no longer reads them. Co-Authored-By: Claude <noreply@anthropic.com>

github-actions Bot added the project: clr label Mar 19, 2026

lamb-j changed the title ~~[CLR] Use CMake-configurable Comgr DLL name instead of runtime versio…~~ [CLR][Windows] Make Comgr DLL name configurable via CMake Mar 19, 2026

systems-assistant Bot added the organization: ROCm label Mar 19, 2026

Merge branch 'develop' into users/lamb-j/rework-comgr-name

b71124f

lamb-j mentioned this pull request Mar 30, 2026

Use COMGR_DLL_NAME via patches to subprojects (workaround for unmerged submodule changes) ROCm/TheRock#4056

Closed

4 tasks

lamb-j marked this pull request as ready for review April 14, 2026 15:10

lamb-j requested review from a team as code owners April 14, 2026 15:10

Merge branch 'develop' into users/lamb-j/rework-comgr-name

8f6c46d

lamb-j mentioned this pull request Apr 14, 2026

Use COMGR_DLL_NAME to configure Windows Comgr DLL naming ROCm/TheRock#4535

Merged

3 tasks

agunashe reviewed Apr 14, 2026

View reviewed changes

Comment thread projects/clr/rocclr/cmake/ROCclrLC.cmake Resolved

Merge branch 'develop' into users/lamb-j/rework-comgr-name

7482e63

agunashe approved these changes Apr 20, 2026

View reviewed changes

lamb-j and others added 2 commits April 20, 2026 16:03

Merge remote-tracking branch 'origin/develop' into users/lamb-j/rewor…

d0877e1

…k-comgr-name

Update TheRock ref to a3e6827

4b273a5

Bumps from 2026-04-07 commit (be15507) to 2026-04-20 commit (a3e6827) to pick up TheRock #4535, which adds COMGR_DLL_NAME wiring needed by this PR's CLR changes. Co-Authored-By: Claude <noreply@anthropic.com>

lamb-j requested a review from a team as a code owner April 20, 2026 23:05

github-actions Bot added the github actions Pull requests that update GitHub Actions code label Apr 20, 2026

rahulc-gh and others added 2 commits April 20, 2026 22:35

Merge branch 'develop' into users/lamb-j/rework-comgr-name

1945b1b

Merge branch 'develop' into users/lamb-j/rework-comgr-name

077c0aa

chrispaquot approved these changes Apr 21, 2026

View reviewed changes

This was referenced Apr 21, 2026

Remove COMGR_DLL_NAME bridging code now that submodules carry the changes ROCm/TheRock#4730

Merged

Update TheRock reference to (a3e68276) #5266

Closed

lamb-j added 2 commits April 21, 2026 16:31

Merge branch 'develop' into users/lamb-j/rework-comgr-name

279537d

Merge branch 'develop' into users/lamb-j/rework-comgr-name

8dad3a6

lamb-j and others added 5 commits April 23, 2026 13:13

Reset rocprofiler workflow files to develop

dc4478a

These files drifted from develop in earlier merges and have nothing to do with the CLR change in this PR. Co-Authored-By: Claude <noreply@anthropic.com>

Merge remote-tracking branch 'origin/develop' into users/lamb-j/rewor…

e25f9b5

…k-comgr-name-no-workflow

Merge remote-tracking branch 'origin/develop' into users/lamb-j/rewor…

b42f5e8

…k-comgr-name-no-workflow

Merge branch 'develop' into users/lamb-j/rework-comgr-name

f29f0dc

Merge branch 'develop' into users/lamb-j/rework-comgr-name

f82e980

lamb-j merged commit 238002a into develop Apr 29, 2026
51 of 56 checks passed

lamb-j deleted the users/lamb-j/rework-comgr-name branch April 29, 2026 16:29

lamb-j mentioned this pull request May 14, 2026

Windows: Use is_versioned = true consistently in both Comgr::LoadLib paths. #493

Closed

zichguan-amd mentioned this pull request Jun 25, 2026

[clr] Use is_versioned = true consistently in both Comgr::LoadLib paths. ROCm/TheRock#628

Closed

This was referenced Jul 29, 2026

Add top-level option to override Comgr DLL name for comgr + CLR builds ROCm/TheRock#6970

Open

feat(build): add THEROCK_COMGR_DLL_NAME to override comgr DLL name ROCm/TheRock#6971

Merged

Navigation Menu

Repository navigation

[CLR][Windows] Make Comgr DLL name configurable via CMake - #4211

[CLR][Windows] Make Comgr DLL name configurable via CMake #4211
lamb-j merged 16 commits into
developfrom
users/lamb-j/rework-comgr-name

lamb-j commented Mar 19, 2026 •

edited

Loading

Uh oh!

lamb-j commented Apr 14, 2026

Uh oh!

Uh oh!

lamb-j commented Apr 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

lamb-j commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

JIRA ID

Test Plan

Test Result

Submission Checklist

Uh oh!

lamb-j commented Apr 14, 2026

Uh oh!

Uh oh!

lamb-j commented Apr 28, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lamb-j commented Mar 19, 2026 •

edited

Loading