MiOpen Unit Test issue on Windows (redefinition of variables)

[saeid-rostami]

This issue happens when MIopen wants to use HIPCC_RTC and standard libraries like #include limits at the same time. In general hipRTC does not allow the inclusion of std headers, however; there are some workaround for some issues that force to use hipRTC and std libraries at the same time and that cause problem.

For example one of them is in naive_conv.cpp, std::integral_constant was defined in standard library and amd_hip_vector_types.h at the same time,

MIOpen(HIP): Error [Compile] 'hiprtcCompileProgram(prog.get(), c_options.size(), c_options.data())' naive_conv.cpp: HIPRTC_ERROR_COMPILATION (6)

MIOpen(HIP): Error [BuildHip] HIPRTC status = HIPRTC_ERROR_COMPILATION (6), source file: naive_conv.cpp

MIOpen(HIP): Warning [BuildHip] In file included from C:\Users\srostami\AppData\Local\Temp\comgr-5f47af\input\naive_conv.cpp:39:

In file included from C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\limits:16:

In file included from C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\xstddef:11:

In file included from C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\cstddef:13:

C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\xtr1common:21:8: error: redefinition of 'integral_constant'

struct integral_constant {

       ^

C:/constructicon/builds/gfx/two/23.10/drivers/compute/hipamd/include\hip/amd_detail/amd_hip_vector_types.h:55:38: note: previous definition is here

template <class _Tp, _Tp __v> struct integral_constant {

There are two places that I found this issue is happening 

1- src/kernels/gpu_reference_kernel/naive_conv.cpp 2- src/composable_kernel/composable_kernel/include/utility/data_type.hpp

Recently, there is an update at the first one (naive_conv.cpp) and a protection was added to that (#ifndef MIOPEN_DONT_USE_HIP_RUNTIME_HEADERS) and this change fixed that issue related at this file

https://github.com/ROCmSoftwarePlatform/MIOpen/blob/68524a8223a6550115589e2bd671d378d92fe470/src/kernels/gpu_reference_kernel/naive_conv.cpp#L39-L41

However, for the second one (data_type.hpp) there is no (#ifndef MIOPEN_DONT_USE_HIP_RUNTIME_HEADERS) and the issue is still there!

https://github.com/ROCmSoftwarePlatform/MIOpen/blob/68524a8223a6550115589e2bd671d378d92fe470/src/composable_kernel/composable_kernel/include/utility/data_type.hpp#L14

I tried to do the same thing that you did for naive_conv.cpp in data_type.hpp and add #ifndef MIOPEN_DONT_USE_HIP_RUNTIME_HEADERS, this change fixed that redefinition compiler error but caused other issues, and that is because of Following lines

https://github.com/ROCmSoftwarePlatform/MIOpen/blob/68524a8223a6550115589e2bd671d378d92fe470/src/composable_kernel/composable_kernel/include/utility/data_type.hpp#L1025-L1035

So, if I added #ifndef MIOPEN_DONT_USE_HIP_RUNTIME_HEADERS I'll get errors because NumericalLimits::min and NumericalLimits::max are being used by other files, if I do not add #ifndef MIOPEN_DONT_USE_HIP_RUNTIME_HEADERS, I'll get redefinition of variables compiler error.

Would you please help on this how these cases are being handled on Linux side and how we can do the same thing on Windows ?

Thanks,

[atamazov]

I can help. Please indicate how urgent and how important is this for you.

[Informative] Please read this https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/creating-a-permanent-link-to-a-code-snippet. This feature allows to use links to the code instead of inserting pictures.

[Recommendation] I would appreciate if you edit your initial comment and replace pics with links.

[saeid-rostami]

Updated.

[saeid-rostami]

The priority is high for us. Thank you!

[atamazov]

@saeid-rostami

There are two places that I found this issue is happening 1- src/kernels/gpu_reference_kernel/naive_conv.cpp

Recently, there is an update at the first one (naive_conv.cpp) and a protection was added to that (#ifndef MIOPEN_DONT_USE_HIP_RUNTIME_HEADERS) and this change fixed that issue related at this file

These two statements contradict. Please clarify do you experience a build issue with naive_conv.cpp.

[saeid-rostami]

I do not have issue with naive_conv.cpp anymore. Adding #ifndef MIOPEN_DONT_USE_HIP_RUNTIME_HEADERS solved issue for naive_conv.cpp but I have the exact the same issue with data_type.hpp.

[atamazov]

AFAICS this is Windows-specific issue. Recommendations/guidelines:

• In the host code (comgr.cpp)
  • introduce macro WORKAROUND_ISSUE_2184. It should be defined as 1 only for Windows (otherwise - 0).
  • #If WORKAROUND_ISSUE_2184
    • In hiprtc::BuildHip(), pass -DWORKAROUND_ISSUE_2184 to the compiler
• In the device code (data_type.hpp)
  • #ifdef WORKAROUND_ISSUE_2184:
    • Do not include <limits> but
      • Either provide definitions of std::numeric_limits<T>::min()/max()/lowest() instead of inclusion of <limits>,
      • Or provide your own specializations of NumericLimits<T>::Min()/Max()/Lowest() instead of what we have now in data_type.hpp, lines 1026-1034.
  • #else
    • The code of data_type.hpp after preprocessing should remain the same.

As an example, please refer to how support for WORKAROUND_ISSUE_HIPRTC_TRUE_TYPE is implemented in #1237.

[ppanchad-amd]

@saeid-rostami Is this still an issue for you? If not, please close. Thanks!

[ppanchad-amd]

@saeid-rostami Closing ticket. Please re-open if you still need assistance with this ticket. Thanks!