Remove COMPILING_TARGETS from CMakeLists.txt #1533
Conversation
sohaibnd
requested review from
wenkaidu,
gilbertlee-amd,
akolliasAMD,
PedramAlizadeh,
nusislam,
nileshnegi,
KawtharShafie,
AtlantaPepsi,
mberenjk,
corey-derochie-amd,
mustafabar,
thananon,
JhaShweta1,
BertanDogancay,
rahulvaidya20,
jee7s,
isaki001,
PJAvinash,
AbandiGa and
haripriya-amd
as code owners
| @@ -83,7 +85,7 @@ if (BUILD_ADDRESS_SANITIZER) | |||
| endif() | |||
| endif() | |||
| endforeach() | |||
| SET(GPU_TARGETS "${amdgpu_targets}" CACHE STRING "Modified GPU list for Address-Sanitizer enabled build." FORCE) | |||
| SET(GPU_TARGETS "${amdgpu_targets}") | |||
There was a problem hiding this comment.
we need FORCE here to override the default list of GPU_TARGETS with xnack+ appended values if not specified
There was a problem hiding this comment.
I've changed it to a normal variable instead of a cache variable (CACHE keyword removed) which does't need FORCE (reference: https://cmake.org/cmake/help/latest/command/set.html). Another advantage is that since normal variables won't be cached, if the user specifies BUILD_ADDRESS_SANITIZER OFF after previously specifying ON, the xnack+ will no longer be appended which is what you expect.
There was a problem hiding this comment.
does not work as intended
There was a problem hiding this comment.
There is a set(GPU_TARGETS...CACHE) which gets called in hip-config-amd.cmake and leads to the normal variable being unset because of CMP0126, so I changed line 71 back to a set(CACHE), it should work now. I also got rid of COMPILING_TARGETS since it is not actually used to set --offload-arch option.
sohaibnd
force-pushed
the
sonadeem/cmake_fix
branch
from
c7aebe2 to
d6aa30b
Compare
sohaibnd
changed the title
COMPILING_TARGETS is not actually used for --offload-arch option, instead GPU_TARGETS is being used implicitly when we call find_package(hip REQUIRED) (See hip-config-amd.cmake).
sohaibnd
force-pushed
the
sonadeem/cmake_fix
branch
from
d6aa30b to
3af27ec
Compare
COMPILING_TARGETS is not actually used for --offload-arch option, instead GPU_TARGETS is being used implicitly when we call
find_package(hip REQUIRED) (See hip-config-amd.cmake)
Details
Do not mention proprietary info or link to internal work items in this PR.
Work item: #1509
What were the changes?
Remove COMPILING_TARGETS from CMakeLists.txt
Why were the changes made?
COMPILING_TARGETS is not actually used for --offload-arch option, instead GPU_TARGETS is being used implicitly when we call
find_package(hip REQUIRED) (See hip-config-amd.cmake)
Approval Checklist
Do not approve until these items are satisfied.