-
Notifications
You must be signed in to change notification settings - Fork 73
amd-device-libs: Respect ROCM_DEVICE_LIBS_BITCODE_INSTALL_LOC_NEW in AMDDeviceLibsConfig.cmake #303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Github action triggered jenkins job: https://compiler-ci.amd.com/job/compiler-upstream-rocm-llvm-project-psdb/131 |
|
Github action triggered jenkins job: https://compiler-ci.amd.com/job/compiler-upstream-rocm-llvm-project-psdb/132 |
|
Please ignore the PSDB verification trigger. IWe are enabling PSDB for amd-staging PR's starting Oct 20. I was checking if the triggers are happening as expected on non test PR's . Sorry if this had interrupted you. |
|
There seems to be a compilation error in the latest build. We have deployed the PSDB automation today. Please check the compilation error. |
Hello @skganesan008 , thanks for the feedback. The log seems at https://compiler-ci.amd.com/job/compiler-psdb-amd-staging/2354, is this accessible from outside AMD? |
|
@traversaro not yet publicly accessible, that's a work in progress though The failure doesn't seem related to your change though: [2025-10-21T16:45:02.922Z] /jenkins/workspace/compiler-psdb-amd-staging/repos/llvm-project/llvm/projects/SPIRV-LLVM-Translator/lib/SPIRV/LLVMToSPIRVDbgTran.cpp:591:31: error: request for member ‘getUnversionedName’ in ‘CU->llvm::DICompileUnit::getSourceLanguage()’, which is of non-class type ‘unsigned int’ Can you rebase on top of the latest amd-staging? That may fix it |
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, good catch
We're already setting INSTALL_ROOT_SUFFIX to amdgcn/bitcode here (as you mentioned):
| set (INSTALL_ROOT_SUFFIX "amdgcn/bitcode") |
Motivation
When the
ROCM_DEVICE_LIBS_BITCODE_INSTALL_LOC_NEWvariable is specified, the install location of bytecode changes, but this change is not reflected in theAMDDeviceLibsConfig.cmakefile, that still points to the default install location. This PR fixes this by respecting theROCM_DEVICE_LIBS_BITCODE_INSTALL_LOC_NEWwhen generating theAMDDeviceLibsConfig.cmakefile.Technical Details
The
Packages.cmakewas redefining ainstall_path_suffixvariable with the hardcoded install value, but a similar variableINSTALL_ROOT_SUFFIX(that respectsROCM_DEVICE_LIBS_BITCODE_INSTALL_LOC_NEWwas already defined inOCL.cmake, so asOCLis included beforePackages, we can just get rid ofinstall_path_suffixand useINSTALL_ROOT_SUFFIX.This removes the need for patches or workarounds used in downstream packaging, see:
Test Plan
I configured the
amd/device-libsfolder in standalone mode, and specified a custom install location withROCM_DEVICE_LIBS_BITCODE_INSTALL_LOC_NEW, and I verified manually thatAMDDeviceLibsConfig.cmakewas correctly updated.Test Result
See Test Plan.
Submission Checklist