[Target] Adopt kernel triple and data layout for device libraries - #2467
ppetrovi-amd wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The packaged-library normalization lacks automated regression coverage.
Review effort: Balanced
Findings: 1
What changed in this PR
Updates AMDGPU device-library linking to suppress LLVM triple and data-layout mismatch warnings.
Changes:
- Applies the kernel module’s target triple and data layout to loaded device libraries.
| File | Description |
|---|---|
mlir/lib/Target/Target.cpp |
Normalizes device-library target metadata before linking. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| for (std::unique_ptr<llvm::Module> &library : bcFiles) { | ||
| library->setTargetTriple(module.getTargetTriple()); | ||
| library->setDataLayout(module.getDataLayout()); |
There was a problem hiding this comment.
Added mlir/test/Target/packaged-device-libs.mlir. It links the packaged OCML through gpu-module-to-binary and checks that no "Linking two modules" warning is printed.
While writing it I found that libraries loaded from disk already go through handleBitcodeFile, which sets our triple and data layout; only the packaged path skipped it. The fix now calls handleBitcodeFile for packaged libraries instead of normalizing every library.
The test only fails on a regression when rocMLIR is built against device libraries from a newer LLVM; with matching libraries there's no mismatch to warn about.
Libraries loaded from disk go through handleBitcodeFile, which gives them our triple and data layout, but packaged device libraries skipped it. When those are built by a newer LLVM than ours (amdgpu-amd-amdhsa triple, extra address spaces in the data layout), the IR linker warned twice for every kernel that calls OCML/OCKL. Run packaged libraries through handleBitcodeFile too. Generated instructions are unchanged. As for libraries loaded from disk, the packaged libraries' llvm.ident and opencl.ocl.version metadata are now dropped, so kernels using OCML/OCKL no longer report OpenCL C as their language.
46f22a9 to
4dd432e
Compare
Motivation
When rocMLIR is built against a ROCm install whose device libraries were compiled by a newer LLVM than rocMLIR's own, every kernel that calls OCML/OCKL prints two IR linker warnings. The libraries use the
amdgpu-amd-amdhsatriple and a data layout with extra address spaces (p10–p15), which do not match the kernel module.Technical Details
Libraries loaded from disk already go through
SerializeGPUModuleBase::handleBitcodeFile, which sets our triple and data layout. The packaged device libraries inAMDGPUSerializer::loadBitcodeFilesskipped it; they now go throughhandleBitcodeFiletoo.Generated instructions are unchanged. As for libraries loaded from disk, the packaged libraries'
llvm.identandopencl.ocl.versionmetadata are now dropped, so kernels using OCML/OCKL no longer report OpenCL C as their language.Test Plan
gfrg-v3-fp32-512x512) with rocMLIR built against a ROCm install with newer device libraries, before and after the change.mlir/test/Target/packaged-device-libs.mlir. Built against newer device libraries, it reproduces both warnings without the fix and passes with it.Test Result
Submission Checklist