From 5cd758368af4e2099596efd0a85100b1a57cdb8e Mon Sep 17 00:00:00 2001 From: Sergey Sharybin Date: Tue, 4 Mar 2025 09:35:54 +0100 Subject: [PATCH] Fix #134992: HIP-RT fails building BVH if Blender path contains space Quite obscure issue, seemingly caused by the fact that HIP-RT is passing a large (about 20 MB) global array to a different library (HIP driver, via hipModuleLoadData). Having global variables of such size seems to be always problematic as they can not be stored on stack and, possibly, extra mapping is involved here. It is not clear whether it is a quirk of the HIP driver, or Linux, or, maybe, something completely different. It is possible to work-around the problem by making a temporary copy of data on heap memory and pass it to the hipModuleLoadData(). This is how other areas are dealing with modules in Blender. This change contains patch against HIP-RT and the new HIP-RT library compiled with the patch. It seems to fix the problem reported in the report. This change does not resolve OIDN on HIP GPU which seems to have the same issue. However, it is not a recent regression and the bug with OIDN GPU denoising can be reproduced using Blender 4.3. Pull Request: https://projects.blender.org/blender/blender/pulls/135403 --- .../build_environment/cmake/hiprt.cmake | 17 ++++++++++++--- .../patches/hiprt_baked_bvh_array.diff | 21 +++++++++++++++++++ .../{hiprt.diff => hiprt_install.diff} | 9 -------- .../patches/hiprt_target_dependency.diff | 13 ++++++++++++ lib/linux_x64 | 2 +- 5 files changed, 49 insertions(+), 13 deletions(-) create mode 100644 build_files/build_environment/patches/hiprt_baked_bvh_array.diff rename build_files/build_environment/patches/{hiprt.diff => hiprt_install.diff} (66%) create mode 100644 build_files/build_environment/patches/hiprt_target_dependency.diff diff --git a/build_files/build_environment/cmake/hiprt.cmake b/build_files/build_environment/cmake/hiprt.cmake index d64310072ea..1ea290f90b4 100644 --- a/build_files/build_environment/cmake/hiprt.cmake +++ b/build_files/build_environment/cmake/hiprt.cmake @@ -40,9 +40,20 @@ ExternalProject_Add(external_hiprt CMAKE_GENERATOR ${PLATFORM_ALT_GENERATOR} PREFIX ${BUILD_DIR}/hiprt - PATCH_COMMAND ${PATCH_CMD} -p 1 -d - ${BUILD_DIR}/hiprt/src/external_hiprt < - ${PATCH_DIR}/hiprt.diff + # hiprt_target_dependency.diff: + # https://github.com/GPUOpen-LibrariesAndSDKs/HIPRT/pull/31 + # hiprt_install.diff: + # https://github.com/GPUOpen-LibrariesAndSDKs/HIPRT/pull/30 + PATCH_COMMAND + ${PATCH_CMD} -p 1 -d + ${BUILD_DIR}/hiprt/src/external_hiprt < + ${PATCH_DIR}/hiprt_target_dependency.diff && + ${PATCH_CMD} -p 1 -d + ${BUILD_DIR}/hiprt/src/external_hiprt < + ${PATCH_DIR}/hiprt_install.diff && + ${PATCH_CMD} -p 1 -d + ${BUILD_DIR}/hiprt/src/external_hiprt < + ${PATCH_DIR}/hiprt_baked_bvh_array.diff CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=${LIBDIR}/hiprt diff --git a/build_files/build_environment/patches/hiprt_baked_bvh_array.diff b/build_files/build_environment/patches/hiprt_baked_bvh_array.diff new file mode 100644 index 00000000000..46c8ca41996 --- /dev/null +++ b/build_files/build_environment/patches/hiprt_baked_bvh_array.diff @@ -0,0 +1,21 @@ +diff --git a/hiprt/impl/Compiler.cpp b/hiprt/impl/Compiler.cpp +index 514667a..509f3f4 100644 +--- a/hiprt/impl/Compiler.cpp ++++ b/hiprt/impl/Compiler.cpp +@@ -776,7 +776,15 @@ oroFunction Compiler::getFunctionFromPrecompiledBinary( const std::string& funcN + // kernel. + if constexpr ( UseBakedCompiledKernel ) + { +- checkOro( oroModuleLoadData( &module, &bvh_build_array_h ) ); ++ // Copy the data into a heap memory. ++ // ++ // Otherwise it seems to be causing issues with access from the HIP SDK when the application binary is ++ // located in a directory with space in it. ++ // ++ // The speculation is that static global memory can not really be megabytes, and access to it requires ++ // mapping of the original file, and this is where things start to go wrong. ++ std::vector binary(bvh_build_array_h, bvh_build_array_h + bvh_build_array_h_size); ++ checkOro( oroModuleLoadData( &module, binary.data() ) ); + } + else + { diff --git a/build_files/build_environment/patches/hiprt.diff b/build_files/build_environment/patches/hiprt_install.diff similarity index 66% rename from build_files/build_environment/patches/hiprt.diff rename to build_files/build_environment/patches/hiprt_install.diff index 19c8d3f54ef..d99eb5145ed 100644 --- a/build_files/build_environment/patches/hiprt.diff +++ b/build_files/build_environment/patches/hiprt_install.diff @@ -2,15 +2,6 @@ diff --git a/CMakeLists.txt b/CMakeLists.txt index 50eb25e..b13d2da 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt -@@ -503,7 +503,7 @@ if ( BAKE_COMPILED_KERNEL ) - - # Create the 'bake_compiled_kernels' project - add_custom_target(bake_compiled_kernels ALL -- DEPENDS ${KERNEL_HIPRT_H} ${KERNEL_OROCHI_H} -+ DEPENDS ${KERNEL_HIPRT_H} ${KERNEL_OROCHI_H} precompile_kernels - ) - - add_dependencies(${HIPRT_NAME} precompile_kernels bake_compiled_kernels) @@ -585,12 +585,16 @@ install(FILES ${HIPRT_ORO_HEADERS} DESTINATION include/contrib/Orochi/ParallelPrimitives) diff --git a/build_files/build_environment/patches/hiprt_target_dependency.diff b/build_files/build_environment/patches/hiprt_target_dependency.diff new file mode 100644 index 00000000000..63383dd4358 --- /dev/null +++ b/build_files/build_environment/patches/hiprt_target_dependency.diff @@ -0,0 +1,13 @@ +diff --git a/CMakeLists.txt b/CMakeLists.txt +index 50eb25e..b13d2da 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -503,7 +503,7 @@ if ( BAKE_COMPILED_KERNEL ) + + # Create the 'bake_compiled_kernels' project + add_custom_target(bake_compiled_kernels ALL +- DEPENDS ${KERNEL_HIPRT_H} ${KERNEL_OROCHI_H} ++ DEPENDS ${KERNEL_HIPRT_H} ${KERNEL_OROCHI_H} precompile_kernels + ) + + add_dependencies(${HIPRT_NAME} precompile_kernels bake_compiled_kernels) diff --git a/lib/linux_x64 b/lib/linux_x64 index eacf548b2e6..3cf676e54a5 160000 --- a/lib/linux_x64 +++ b/lib/linux_x64 @@ -1 +1 @@ -Subproject commit eacf548b2e6c10d1a6d5879218ecaa11f102310a +Subproject commit 3cf676e54a5be98285c6d47633fbe1f26929bb5a