From c388ed1e5312f19491be2a9459a9f41449956a04 Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Mon, 11 Mar 2024 15:09:24 +0100 Subject: [PATCH] Fix #118709: Crash in OIDN GPU detection for unsupported HIP device Pull Request: https://projects.blender.org/blender/blender/pulls/119315 --- build_files/build_environment/patches/oidn.diff | 21 +++++++++++++++++++++ intern/cycles/device/hip/device.cpp | 4 +++- intern/cycles/device/hip/device_impl.cpp | 17 +++++------------ intern/cycles/device/hip/util.h | 18 ++++++++++++++++++ intern/cycles/device/hiprt/device_impl.cpp | 17 ++++++----------- 5 files changed, 53 insertions(+), 24 deletions(-) diff --git a/build_files/build_environment/patches/oidn.diff b/build_files/build_environment/patches/oidn.diff index 782bddcd9c1..77ad933a655 100644 --- a/build_files/build_environment/patches/oidn.diff +++ b/build_files/build_environment/patches/oidn.diff @@ -52,3 +52,24 @@ diff -Naur oidn-2.2.0/devices/CMakeLists.txt external_openimagedenoise/devices/C BUILD_ALWAYS TRUE DEPENDS OpenImageDenoise_core +diff --git a/devices/hip/hip_device.cpp b/devices/hip/hip_device.cpp +index ae14ced..a49e131 100644 +--- a/devices/hip/hip_device.cpp ++++ b/devices/hip/hip_device.cpp +@@ -93,10 +93,16 @@ OIDN_NAMESPACE_BEGIN + { + const std::string name = getArchName(prop); + ++ // BLENDER: this comment is meant to generate a merge conflict if the code ++ // here changes, so we know that hipSupportsDeviceOIDN should be updated. + if (name == "gfx1030") + return HIPArch::DL; ++ // BLENDER: this comment is meant to generate a merge conflict if the code ++ // here changes, so we know that hipSupportsDeviceOIDN should be updated. + if (name == "gfx1100" || name == "gfx1101" || name == "gfx1102") + return HIPArch::WMMA; ++ // BLENDER: this comment is meant to generate a merge conflict if the code ++ // here changes, so we know that hipSupportsDeviceOIDN should be updated. + else + return HIPArch::Unknown; + } diff --git a/intern/cycles/device/hip/device.cpp b/intern/cycles/device/hip/device.cpp index 35e369364ee..62730e06b06 100644 --- a/intern/cycles/device/hip/device.cpp +++ b/intern/cycles/device/hip/device.cpp @@ -184,7 +184,9 @@ void device_hip_info(vector &devices) info.denoisers = 0; # if defined(WITH_OPENIMAGEDENOISE) - if (OIDNDenoiserGPU::is_device_supported(info)) { + /* Check first if OIDN supports it, not doing so can crash the HIP driver with + * "hipErrorNoBinaryForGpu: Unable to find code object for all current devices". */ + if (hipSupportsDeviceOIDN(num) && OIDNDenoiserGPU::is_device_supported(info)) { info.denoisers |= DENOISER_OPENIMAGEDENOISE; } # endif diff --git a/intern/cycles/device/hip/device_impl.cpp b/intern/cycles/device/hip/device_impl.cpp index 9daf48dec34..44dc37c2055 100644 --- a/intern/cycles/device/hip/device_impl.cpp +++ b/intern/cycles/device/hip/device_impl.cpp @@ -227,19 +227,11 @@ string HIPDevice::compile_kernel(const uint kernel_features, const char *name, c int major, minor; hipDeviceGetAttribute(&major, hipDeviceAttributeComputeCapabilityMajor, hipDevId); hipDeviceGetAttribute(&minor, hipDeviceAttributeComputeCapabilityMinor, hipDevId); - hipDeviceProp_t props; - hipGetDeviceProperties(&props, hipDevId); - - /* gcnArchName can contain tokens after the arch name with features, ie. - * `gfx1010:sramecc-:xnack-` so we tokenize it to get the first part. */ - char *arch = strtok(props.gcnArchName, ":"); - if (arch == NULL) { - arch = props.gcnArchName; - } + const std::string arch = hipDeviceArch(hipDevId); /* Attempt to use kernel provided with Blender. */ if (!use_adaptive_compilation()) { - const string fatbin = path_get(string_printf("lib/%s_%s.fatbin", name, arch)); + const string fatbin = path_get(string_printf("lib/%s_%s.fatbin", name, arch.c_str())); VLOG_INFO << "Testing for pre-compiled kernel " << fatbin << "."; if (path_exists(fatbin)) { VLOG_INFO << "Using precompiled kernel."; @@ -267,10 +259,11 @@ string HIPDevice::compile_kernel(const uint kernel_features, const char *name, c # ifndef NDEBUG options.append(" -save-temps"); # endif - options.append(" --amdgpu-target=").append(arch); + options.append(" --amdgpu-target=").append(arch.c_str()); const string include_path = source_path; - const string fatbin_file = string_printf("cycles_%s_%s_%s", name, arch, kernel_md5.c_str()); + const string fatbin_file = string_printf( + "cycles_%s_%s_%s", name, arch.c_str(), kernel_md5.c_str()); const string fatbin = path_cache_get(path_join("kernels", fatbin_file)); VLOG_INFO << "Testing for locally compiled kernel " << fatbin << "."; if (path_exists(fatbin)) { diff --git a/intern/cycles/device/hip/util.h b/intern/cycles/device/hip/util.h index f1c05d2bb50..af543f5eccb 100644 --- a/intern/cycles/device/hip/util.h +++ b/intern/cycles/device/hip/util.h @@ -4,6 +4,9 @@ #pragma once +#include +#include + #ifdef WITH_HIP # ifdef WITH_HIP_DYNLOAD @@ -46,6 +49,14 @@ const char *hipewCompilerPath(); int hipewCompilerVersion(); # endif /* WITH_HIP_DYNLOAD */ +static std::string hipDeviceArch(const int hipDevId) +{ + hipDeviceProp_t props; + hipGetDeviceProperties(&props, hipDevId); + const char *arch = strtok(props.gcnArchName, ":"); + return (arch == nullptr) ? props.gcnArchName : arch; +} + static inline bool hipSupportsDevice(const int hipDevId) { int major, minor; @@ -55,6 +66,13 @@ static inline bool hipSupportsDevice(const int hipDevId) return (major >= 9); } +static inline bool hipSupportsDeviceOIDN(const int hipDevId) +{ + /* Matches HIPDevice::getArch in HIP. */ + const std::string arch = hipDeviceArch(hipDevId); + return (arch == "gfx1030" || arch == "gfx1100" || arch == "gfx1101" || arch == "gfx1102"); +} + CCL_NAMESPACE_END #endif /* WITH_HIP */ diff --git a/intern/cycles/device/hiprt/device_impl.cpp b/intern/cycles/device/hiprt/device_impl.cpp index f3ef48089f0..bd723c97d04 100644 --- a/intern/cycles/device/hiprt/device_impl.cpp +++ b/intern/cycles/device/hiprt/device_impl.cpp @@ -138,13 +138,7 @@ string HIPRTDevice::compile_kernel(const uint kernel_features, const char *name, int major, minor; hipDeviceGetAttribute(&major, hipDeviceAttributeComputeCapabilityMajor, hipDevId); hipDeviceGetAttribute(&minor, hipDeviceAttributeComputeCapabilityMinor, hipDevId); - hipDeviceProp_t props; - hipGetDeviceProperties(&props, hipDevId); - - char *arch = strtok(props.gcnArchName, ":"); - if (arch == NULL) { - arch = props.gcnArchName; - } + const std::string arch = hipDeviceArch(hipDevId); if (!use_adaptive_compilation()) { const string fatbin = path_get(string_printf("lib/%s_rt_gfx.hipfb", name)); @@ -162,10 +156,11 @@ string HIPRTDevice::compile_kernel(const uint kernel_features, const char *name, const string kernel_md5 = util_md5_string(source_md5 + common_cflags); const string include_path = source_path; - const string bitcode_file = string_printf("cycles_%s_%s_%s.bc", name, arch, kernel_md5.c_str()); + const string bitcode_file = string_printf( + "cycles_%s_%s_%s.bc", name, arch.c_str(), kernel_md5.c_str()); const string bitcode = path_cache_get(path_join("kernels", bitcode_file)); const string fatbin_file = string_printf( - "cycles_%s_%s_%s.hipfb", name, arch, kernel_md5.c_str()); + "cycles_%s_%s_%s.hipfb", name, arch.c_str(), kernel_md5.c_str()); const string fatbin = path_cache_get(path_join("kernels", fatbin_file)); VLOG(1) << "Testing for locally compiled kernel " << fatbin << "."; @@ -229,7 +224,7 @@ string HIPRTDevice::compile_kernel(const uint kernel_features, const char *name, std::string rtc_options; - rtc_options.append(" --offload-arch=").append(arch); + rtc_options.append(" --offload-arch=").append(arch.c_str()); rtc_options.append(" -D __HIPRT__"); rtc_options.append(" -ffast-math -O3 -std=c++17"); rtc_options.append(" -fgpu-rdc -c --gpu-bundle-output -c -emit-llvm"); @@ -260,7 +255,7 @@ string HIPRTDevice::compile_kernel(const uint kernel_features, const char *name, // After compilation, the bitcode produced is linked with HIP RT bitcode (containing // implementations of HIP RT functions, e.g. traversal, to produce the final executable code string linker_options; - linker_options.append(" --offload-arch=").append(arch); + linker_options.append(" --offload-arch=").append(arch.c_str()); linker_options.append(" -fgpu-rdc --hip-link --cuda-device-only "); string hiprt_ver(HIPRT_VERSION_STR); string hiprt_bc = hiprt_path + "\\dist\\bin\\Release\\hiprt" + hiprt_ver + "_amd_lib_win.bc"; -- 2.11.4.GIT