Skip to content

fix(hotswap): gate entry trampoline rewrites for gfx12.5#7593

Merged
ammarwa merged 4 commits into
ROCm:developfrom
harsh-amd:hotswap-entry-trampoline-gating
Jul 1, 2026
Merged

fix(hotswap): gate entry trampoline rewrites for gfx12.5#7593
ammarwa merged 4 commits into
ROCm:developfrom
harsh-amd:hotswap-entry-trampoline-gating

Conversation

@harsh-amd

@harsh-amd harsh-amd commented Jun 22, 2026 •
edited by chinmaydd
Loading

Copy link
Copy Markdown
Contributor

JIRA ID

JIRA ID - ROCM-27304

Stack

Layer 2 of the rocm-systems HotSwap stack, paired with ROCm/llvm-project#3008.

Summary

  • Keep rocm-systems limited to loader routing and COMGR source/target ISA-pair construction; COMGR owns validation and all code-object transformations after the call.
  • Use AMDGPU: add hotswap entry trampoline core llvm-project#3008's explicit COMGR options API: ROCr calls amd_comgr_hotswap_rewrite_with_options with AMD_COMGR_HOTSWAP_REWRITE_FLAG_ENTRY_TRAMPOLINES when it requests entry trampolines, and keeps amd_comgr_hotswap_rewrite for legacy/default rewrites.
  • Keep AMD_COMGR_HOTSWAP_ENTRY_TRAMPOLINES as the ROCr policy knob: default enabled in ROCr, literal 0 disables, unset/empty/any other value enables the entry-trampoline request.
  • Cover concrete gfx125* targets and gfx12-5-generic while keeping non-gfx12.5 agents and source code objects unchanged.
  • Follow Install Kernel Trampolines #7581 by keeping the entry-trampoline path keyed to the code-object ISA rather than using this path for processor retargeting.
  • Preserve the existing gfx1250 A0 source/target ISA pair and leave pass selection to COMGR.
  • Add unit and loader-path coverage for the local routing behavior without testing COMGR pass internals.

Testing

  • git diff --check
  • cmake --build build-rocrtst-hotswap-review --target hotswap_rewrite -j 8
  • ctest --test-dir build-rocrtst-hotswap-review -R hotswap --output-on-failure
  • cmake --build build-rocr-hotswap --target hsa-runtime64 -j 8

This was referenced Jun 22, 2026
Closed
Closed
@harsh-amd harsh-amd force-pushed the hotswap-entry-trampoline-gating branch from 2730218 to 8176f14 Compare June 22, 2026 13:25
@harsh-amd harsh-amd force-pushed the hotswap-entry-trampoline-gating branch from 8176f14 to ad38f93 Compare June 26, 2026 20:01
@therock-pr-bot

therock-pr-bot Bot commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
🌿 Branch Name ✅ Pass —
📝 PR Title/Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

@therock-pr-bot

therock-pr-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Title/Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

@harsh-amd harsh-amd marked this pull request as ready for review June 27, 2026 17:22
@harsh-amd harsh-amd changed the title [hotswap] Gate entry trampoline rewrites explicitly fix(hotswap): gate entry trampoline rewrites for gfx12.5 Jun 27, 2026
@harsh-amd harsh-amd force-pushed the hotswap-entry-trampoline-gating branch from b5f054a to 4b8391b Compare June 29, 2026 23:58
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Jun 29, 2026
@harsh-amd harsh-amd force-pushed the hotswap-entry-trampoline-gating branch 5 times, most recently from 6d68f08 to 65b116f Compare June 30, 2026 14:36
@harsh-amd harsh-amd force-pushed the hotswap-entry-trampoline-gating branch from 65b116f to bbfeb60 Compare June 30, 2026 16:55
shwetagkhatri
shwetagkhatri reviewed Jun 30, 2026
View reviewed changes
Comment thread projects/rocr-runtime/runtime/hsa-runtime/core/runtime/hotswap.cpp Outdated
Comment thread projects/rocr-runtime/runtime/hsa-runtime/core/runtime/hotswap.cpp
@shwetagkhatri shwetagkhatri self-requested a review June 30, 2026 18:53
cfreeamd
cfreeamd reviewed Jun 30, 2026
View reviewed changes

@cfreeamd cfreeamd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few comments for consideration--not blocking.

I noticed this is tagged not ready for review. Will approve when it's ready assuming no significant changes requiring re-review.

Comment thread projects/rocr-runtime/rocrtst/suites/test_common/CMakeLists.txt
Comment thread projects/rocr-runtime/rocrtst/suites/functional/hotswap/hotswap_rewrite_test.cc Outdated
@harsh-amd

harsh-amd commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Added a few comments for consideration--not blocking.

I noticed this is tagged not ready for review. Will approve when it's ready assuming no significant changes requiring re-review.

I am not sure why this is tagged not ready for review. It is ready for review. Do you know how I can get rid of the tag?

@chinmaydd

chinmaydd commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

I am not sure why this is tagged not ready for review. It is ready for review. Do you know how I can get rid of the tag?

I think we need to fix the PR title/description to keep the bot happy.

@harsh-amd

harsh-amd commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

I am not sure why this is tagged not ready for review. It is ready for review. Do you know how I can get rid of the tag?

I think we need to fix the PR title/description to keep the bot happy.

The PR description is as per the bot's requirements. I know its asking for a JIRA ticket, but if you look here #7921 adding a JIRA ticket didn't remove the label here as well.

@chinmaydd

chinmaydd commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

I see. In that case I think its tripping over the "Unit test" check ? Maybe a latent bug.

The following policy checks failed:

❌ Unit Test
The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

EDIT: Updated description with JIRA ID, FTFY

@harsh-amd

harsh-amd commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I see. In that case I think its tripping over the "Unit test" check ? Maybe a latent bug.

The following policy checks failed:

❌ Unit Test
The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

EDIT: Updated description with JIRA ID, FTFY

Thanks!

shwetagkhatri
shwetagkhatri approved these changes Jun 30, 2026
View reviewed changes

@shwetagkhatri shwetagkhatri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ammarwa ammarwa merged commit 746c7b3 into ROCm:develop Jul 1, 2026
34 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kentrussell kentrussell kentrussell approved these changes
@shwetagkhatri shwetagkhatri shwetagkhatri approved these changes
@cfreeamd cfreeamd cfreeamd approved these changes
@pmoutsias-amd pmoutsias-amd pmoutsias-amd approved these changes
@dayatsin-amd dayatsin-amd Awaiting requested review from dayatsin-amd dayatsin-amd is a code owner
@atgutier atgutier Awaiting requested review from atgutier atgutier is a code owner

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation organization: ROCm project: rocr-runtime

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@harsh-amd @chinmaydd @kentrussell @shwetagkhatri @cfreeamd @pmoutsias-amd @ammarwa