Skip to content

test(rocke): fall back to configured Clang for DWARF objects - #11698

Merged
tenpercent merged 3 commits into
developfrom
users/mpodkory/rocke/aick-2167
Sep 14, 2026
Merged

tenpercent merged 3 commits into
developfrom
users/mpodkory/rocke/aick-2167

Conversation

@tenpercent

@tenpercent tenpercent commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

The object/DWARF round-trip tests required llc, so a configured ROCm toolchain with Clang but no llc could select an unrelated driver and mix LLVM versions. The tests now use the first configured directory containing either driver, preferring llc over Clang within that directory. For example, Clang in ROCKE_TEST_LLVM_BIN takes precedence over llc under ROCM_PATH.

ISSUE ID : AICK-2167

Technical Details

  • Share the configured directory order between tool discovery helpers: ROCKE_TEST_LLVM_BIN, $ROCM_PATH/llvm/bin, then $ROCM_PATH/lib/llvm/bin.
  • Try llc, then Clang, within each configured directory before proceeding to the next. Retain the existing PATH/default fallback when no configured driver is available.
  • Preserve the existing llc object-generation command. The Clang command uses --target=amdgcn-amd-amdhsa, -mcpu=<arch>, -nogpulib, and the bitcode input directly.
  • Allow object round-trip tests to proceed when either code-generation driver is available.

This changes test infrastructure only. Production kernel compilation continues to use in-process COMGR.

Test Plan

  • Complete debug-info test file under ROCm 7.14: 56 passed, 7 skipped. The skips require the unavailable C++ engine extension.
  • All four object/DWARF round-trip tests passed using configured Clang with no configured llc.
  • Regression with both environment variables set: explicit-bin Clang wins over ROCm llc; assembler and DWARF dumper also select the explicit bin.
  • Regression for Clang in llvm/bin taking precedence over llc in lib/llvm/bin.
  • Both new precedence regressions failed against the old implementation and passed after the fix.
  • Fall through configured directories without either driver; prefer llc within one directory; prefer configured Clang over unrelated PATH llc.
  • Both ROCm directory layouts and exact llc/Clang command construction.
  • Repository pre-commit checks over the PR range.
  • No GPU or torch required.

Test Result

The configured-Clang object tests preserve subprograms, inline subroutines, call-site file and line information, and abstract origins. Capture-off objects contain no debug-info section.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

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

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@tenpercent
tenpercent marked this pull request as ready for review September 8, 2026 18:42
@tenpercent
tenpercent requested review from a team as code owners September 8, 2026 18:42
@tenpercent
tenpercent requested a review from CongMa13 September 14, 2026 02:55

@CongMa13 CongMa13 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

@tenpercent
tenpercent enabled auto-merge (squash) September 14, 2026 18:11
@tenpercent
tenpercent merged commit 0ba91d0 into develop Sep 14, 2026
193 of 196 checks passed
@tenpercent
tenpercent deleted the users/mpodkory/rocke/aick-2167 branch September 14, 2026 19:08
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants