[googletest][rocm-cmake] multiple ROCMChecks WARNING messages

[junliume]

Issues may be observed when users are building MIOpen

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_FLAGS' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_FLAGS=""'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_FLAGS' is set at
  /home/junliu/MIOpen/build/_deps/googletest-src/googletest/CMakeLists.txt:<line#>
  shown below:
Call Stack (most recent call first):
  build/_deps/googletest-src/googletest/cmake/internal_utils.cmake:9223372036854775807 (rocm_check_toolchain_var)
  build/_deps/googletest-src/googletest/cmake/internal_utils.cmake:42 (string)
  build/_deps/googletest-src/googletest/cmake/internal_utils.cmake:72 (fix_default_compiler_settings_)
  build/_deps/googletest-src/googletest/CMakeLists.txt:83 (config_compiler_and_linker)

*-----------------------------------------------------------------------------*
*******************************************************************************

*******************************************************************************
*------------------------------- ROCMChecks WARNING --------------------------*
  Options and properties should be set on a cmake target where possible. The
  variable 'CMAKE_CXX_FLAGS' may be set by the cmake toolchain, either by
  calling 'cmake -DCMAKE_CXX_FLAGS=""'
  or set in a toolchain file and added with
  'cmake -DCMAKE_TOOLCHAIN_FILE=<toolchain-file>'. ROCMChecks now calling:
CMake Warning at /opt/rocm/share/rocm/cmake/ROCMChecks.cmake:46 (message):
  'CMAKE_CXX_FLAGS' is set at
  /home/junliu/MIOpen/build/_deps/googletest-src/googletest/CMakeLists.txt:<line#>
  shown below:
Call Stack (most recent call first):
  build/_deps/googletest-src/googletest/cmake/internal_utils.cmake:9223372036854775807 (rocm_check_toolchain_var)
  build/_deps/googletest-src/googletest/cmake/internal_utils.cmake:47 (string)
  build/_deps/googletest-src/googletest/cmake/internal_utils.cmake:72 (fix_default_compiler_settings_)
  build/_deps/googletest-src/googletest/CMakeLists.txt:83 (config_compiler_and_linker)

*-----------------------------------------------------------------------------*
*******************************************************************************

[Actual Issue Tracked Here]: https://github.com/RadeonOpenCompute/rocm-cmake/issues/154

[junliume]

Credit to @lawruble13

You can totally suppress the warning by setting the CMake cache variable ROCM_WARN_TOOLCHAIN_VAR to OFF prior to including rocm-cmake, but note that this also suppresses the warning for your own CMake as well (i.e. suppresses any true positives). At present, there is no way to temporarily/partially disable the toolchain variable warnings

[pfultz2]

Ideally, we shouldn't be building googletest in our cmake. It should be listed under requirements.txt(just add google/googletest@v1.14.0) so rbuild/cget can build it once in the dockerfile instead of downloading and rebuilding this everytime.

This also avoids other issues in the cmake such as FORCE overwriting BUILD_SHARED_LIBS to Off, which can make a different ordering of includes in cmake always build miopen as static.

My original review feedback for this was ignored, but is still relevant:

https://github.com/ROCmSoftwarePlatform/MIOpen/pull/1611/files#r912193677

[junliume]

@pfultz2 thanks! The original comment was not ignored, while we thought this is rather standard and officially recommended way of including gooletest, e.g. also in Triton: https://github.com/ROCmSoftwarePlatform/triton/blob/9517d4c2566cd2763d038b95b79713a04a36eac6/unittest/googletest.cmake#L10 I agree that it is not ideal that we rebuild this everytime.

Maybe ROCm should also have included googletest as a dependency to avoid too many different versions among different components.

[junliume]

@xinlipn could you follow up with the above comment, and next time discuss how the way googletest should be installed?

[apwojcik]

According to CMake Dependency Guide the primary methods of bringing dependencies into the build are the find_package() command and the FetchContent module. The ROCm-CMake ROCMCheck is too restrictive on third-party projects. However, that can resolved by using the SYSTEM attribute for FetchContent_Declare().