From d863885373a9ca5d511167dad691894330f44679 Mon Sep 17 00:00:00 2001 From: baldurk Date: Thu, 1 Oct 2020 17:40:05 +0100 Subject: [PATCH] Disable KHR_buffer_device_address on AMD windows for now * It seems to be broken at the moment unfortunately, at least for our internal use cases (shader debugging and bindless feedback) --- renderdoc/core/settings.h | 12 +++++++++--- renderdoc/driver/vulkan/vk_bindless_feedback.cpp | 5 +++++ renderdoc/driver/vulkan/vk_common.cpp | 6 ++++++ renderdoc/driver/vulkan/vk_common.h | 4 ++++ renderdoc/driver/vulkan/vk_shaderdebug.cpp | 3 ++- 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/renderdoc/core/settings.h b/renderdoc/core/settings.h index 661a943ec..63640c300 100644 --- a/renderdoc/core/settings.h +++ b/renderdoc/core/settings.h @@ -58,7 +58,9 @@ CONFIG_SUPPORT_TYPE(rdcarray); #define RDOC_CONFIG(type, name, defaultValue, description) \ static ConfigVarRegistration CONCAT(config, __LINE__)( \ STRING_LITERAL(STRINGIZE(name)), defaultValue, false, STRING_LITERAL(description)); \ - static const type &name() { return CONCAT(config, __LINE__).value(); } + const type &name() { return CONCAT(config, __LINE__).value(); } +#define RDOC_EXTERN_CONFIG(type, name) extern const type &name(); + // debug configs get set to constants in official stable builds, they will remain configurable // in nightly builds and of course in development builds #if RENDERDOC_STABLE_BUILD @@ -66,11 +68,15 @@ CONFIG_SUPPORT_TYPE(rdcarray); #define RDOC_DEBUG_CONFIG(type, name, defaultValue, description) \ static ConfigVarRegistration CONCAT(config, __LINE__)( \ STRING_LITERAL(STRINGIZE(name)), defaultValue, true, STRING_LITERAL(description)); \ - static type name() { return defaultValue; } + const type &name() \ + { \ + static const type ret = defaultValue; \ + return ret; \ + } #else #define RDOC_DEBUG_CONFIG(type, name, defaultValue, description) \ static ConfigVarRegistration CONCAT(config, __LINE__)( \ STRING_LITERAL(STRINGIZE(name)), defaultValue, true, STRING_LITERAL(description)); \ - static const type &name() { return CONCAT(config, __LINE__).value(); } + const type &name() { return CONCAT(config, __LINE__).value(); } #endif diff --git a/renderdoc/driver/vulkan/vk_bindless_feedback.cpp b/renderdoc/driver/vulkan/vk_bindless_feedback.cpp index 200d4bdc7..741e0ef44 100644 --- a/renderdoc/driver/vulkan/vk_bindless_feedback.cpp +++ b/renderdoc/driver/vulkan/vk_bindless_feedback.cpp @@ -36,6 +36,7 @@ RDOC_DEBUG_CONFIG(rdcstr, Vulkan_Debug_FeedbackDumpDirPath, "", RDOC_CONFIG( bool, Vulkan_BindlessFeedback, true, "Enable fetching from GPU which descriptors were dynamically used in descriptor arrays."); +RDOC_EXTERN_CONFIG(bool, Vulkan_Debug_DisableBufferDeviceAddress); struct feedbackData { @@ -551,6 +552,10 @@ void VulkanReplay::FetchShaderFeedback(uint32_t eventId) m_pDriver->GetExtensions(NULL).ext_EXT_buffer_device_address) && m_pDriver->GetDeviceEnabledFeatures().shaderInt64; + if(Vulkan_Debug_DisableBufferDeviceAddress() || + m_pDriver->GetDriverInfo().AMDBufferDeviceAddressBrokenDriver()) + useBufferAddress = false; + bool useBufferAddressKHR = m_pDriver->GetExtensions(NULL).ext_KHR_buffer_device_address; const VulkanRenderState &state = m_pDriver->m_RenderState; diff --git a/renderdoc/driver/vulkan/vk_common.cpp b/renderdoc/driver/vulkan/vk_common.cpp index 75321ce58..f88d2beca 100644 --- a/renderdoc/driver/vulkan/vk_common.cpp +++ b/renderdoc/driver/vulkan/vk_common.cpp @@ -970,6 +970,12 @@ VkDriverInfo::VkDriverInfo(const VkPhysicalDeviceProperties &physProps) if(physProps.driverVersion < VK_MAKE_VERSION(2, 0, 33)) amdStorageMSAABrokenDriver = true; } + + if(m_Vendor == GPUVendor::AMD) + { + // not yet fixed + amdBDABrokenDriver = true; + } #endif // not fixed yet diff --git a/renderdoc/driver/vulkan/vk_common.h b/renderdoc/driver/vulkan/vk_common.h index e1a250535..367d5ea95 100644 --- a/renderdoc/driver/vulkan/vk_common.h +++ b/renderdoc/driver/vulkan/vk_common.h @@ -253,6 +253,9 @@ public: // On Qualcomm emitting an image sample operation with DRef and explicit lod will crash on non-2D // textures. Since 2D is the common/expected case, we avoid compiling that case entirely. bool QualcommDrefNon2DCompileCrash() const { return qualcommDrefNon2DCompileCrash; } + // On AMD unfortunately the initial implementation of KHR_buffer_device_address is broken and + // produces bad results. + bool AMDBufferDeviceAddressBrokenDriver() const { return amdBDABrokenDriver; } private: GPUVendor m_Vendor; @@ -264,6 +267,7 @@ private: bool amdStorageMSAABrokenDriver = false; bool qualcommLeakingUBOOffsets = false; bool qualcommDrefNon2DCompileCrash = false; + bool amdBDABrokenDriver = false; }; enum diff --git a/renderdoc/driver/vulkan/vk_shaderdebug.cpp b/renderdoc/driver/vulkan/vk_shaderdebug.cpp index 4e059aaba..2e71aefc3 100644 --- a/renderdoc/driver/vulkan/vk_shaderdebug.cpp +++ b/renderdoc/driver/vulkan/vk_shaderdebug.cpp @@ -3787,7 +3787,8 @@ ShaderDebugTrace *VulkanReplay::DebugPixel(uint32_t eventId, uint32_t x, uint32_ } } - if(Vulkan_Debug_DisableBufferDeviceAddress()) + if(Vulkan_Debug_DisableBufferDeviceAddress() || + m_pDriver->GetDriverInfo().AMDBufferDeviceAddressBrokenDriver()) storageMode = Binding; rdcarray fragspv = shader.spirv.GetSPIRV();