Skip to content
This repository has been archived by the owner on Jan 26, 2024. It is now read-only.

AMD_COMGR_ACTION_ADD_DEVICE_LIBRARIES is deprecated #41

Open
aaronmondal opened this issue Apr 11, 2023 · 3 comments
Open

AMD_COMGR_ACTION_ADD_DEVICE_LIBRARIES is deprecated #41

aaronmondal opened this issue Apr 11, 2023 · 3 comments

Comments

@aaronmondal
Copy link

...and a very long macro name 😂

This might have been overlooked previously since the warning wasn't properly printed when building with Clang before ROCm/ROCm-CompilerSupport@55deecf.

This also affects a few similar symbols. Warnings look like this:

external/rules_ll~override~rules_ll_dependencies~comgr/lib/comgr/src/comgr.cpp:209:8: warning: 'AMD_COMGR_ACTION_ADD_DEVICE_LIBRARIES' is deprecated: Will be removed in Comgr
 v3.0 (Rocm v6.0). Use AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC instead [-Wdeprecated-declarations]
  case AMD_COMGR_ACTION_ADD_DEVICE_LIBRARIES:
       ^
bazel-out/k8-fastbuild/bin/external/rules_ll~override~rules_ll_dependencies~comgr/comgr/amd_comgr/amd_comgr.h:1560:3: note: 'AMD_COMGR_ACTION_ADD_DEVICE_LIBRARIES' has been e
xplicitly marked deprecated here
  AMD_COMGR_DEPRECATED("Will be removed in Comgr v3.0 (Rocm v6.0). Use "
  ^
bazel-out/k8-fastbuild/bin/external/rules_ll~override~rules_ll_dependencies~comgr/comgr/amd_comgr/amd_comgr.h:55:50: note: expanded from macro 'AMD_COMGR_DEPRECATED'
#define AMD_COMGR_DEPRECATED(msg) __attribute__((deprecated(msg)))
                                                 ^
external/rules_ll~override~rules_ll_dependencies~comgr/lib/comgr/src/comgr.cpp:231:8: warning: 'AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN' is deprecated: Will be removed in C
omgr v3.0 (Rocm v6.0). Use AMD_COMGR_ACTION_COMPILE_SOURCE_TO_BC, etc. instead [-Wdeprecated-declarations]
  case AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN:
       ^
bazel-out/k8-fastbuild/bin/external/rules_ll~override~rules_ll_dependencies~comgr/comgr/amd_comgr/amd_comgr.h:1711:3: note: 'AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN' has be
en explicitly marked deprecated here
  AMD_COMGR_DEPRECATED("Will be removed in Comgr v3.0 (Rocm v6.0). Use "
  ^

I tried naively updating the macros but that seems to break everything, so I guess we need the expertise of someone who actually knows what they are doing here 😇

cc @lamb-j

@lamb-j
Copy link
Contributor

lamb-j commented Apr 11, 2023

Those warnings seem correct. So you're having issues moving away from the following deprecated (soon to be removed) actions?

AMD_COMGR_ACTION_ADD_DEVICE_LIBRARIES 
AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN

What kind of issues are you having? There is still one known bug with the updated AMD_COMGR_ACTION_COMPILE_SOURCE_WITH_DEVICE_LIBS_TO_BC action for OpenCL compilations, but I don't know of any issues moving away from AMD_COMGR_ACTION_COMPILE_SOURCE_TO_FATBIN.

@aaronmondal
Copy link
Author

Yes the warnings are correct. I'm just worried that ROCclr hasn't updated this yet since technically these symbols have been deprecated for quite some time and are still in the current main branch.

I'ts been a while since I played around with this, but I remember not actually getting any compiletime (non-amd clang) or linktime (clang-linker-wrapper with lld and offloading to amdgpu) errors but runtime errors where code would just not compute anything and wouldn't raise any errors.

@lamb-j
Copy link
Contributor

lamb-j commented Apr 11, 2023

Ah ok I understand. I've actually submitted a patch to remove the ADD_DEVICE_LIBRARY calls, but had to revert it, possibly due to the same issues you saw:

SWDEV-376413 - Replace deprecated Comgr device-lib action
SWDEV-376413 - Revert "SWDEV-376413 - Replace deprecated Comgr device…

I'm currently working on re-applying this patch with updates to fix the errors.

I'm not sure where the SOURCE_TO_FATBIN warning is coming from though, I don't see that being used in the ROCclr repo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants