vvl: Rework accel structs mem aliasing validation #9492

arno-lunarg · 2025-02-20T14:21:25Z

In acceleration structures build commands, when looking for overlaps between source acceleration structures, destination acceleration structures, and scratch buffers, do it using device address ranges instead of relying on the device memory objects, and sort device address ranges.
=> Since overlap detection is done when a new device address range is inserted among the previously seen ones, and insertion is done using dichotomy (faster and ok since address ranges are sorted), not all overlap will be detected, but if there is any, it will be detected.
For instance, if all ranges are equal, inserting yet another equal range will likely result in it being directly placed at the beginning or end of the list, with only one overlap detected with first of last element.
Given the performance gains, and the fact that most calls done by apps to vkCmdBuildAccelerationStructuresKHR are expected to be valid so no memory overlaps will be detected, this compromise is well worth it.

Doing some stress test, where I build 8000 acceleration structures in one go, PreCallValidateCmdBuildAccelerationStructuresKHR went from ~4 minutes down to ~200 ms, and the overlap validation itself is about ~100 ms

Still need to do the same change at other spots, but this one in particular was the issue, so let's get this in first

ci-tester-lunarg · 2025-02-20T14:21:29Z

CI Vulkan-ValidationLayers build queued with queue ID 376152.

ci-tester-lunarg · 2025-02-20T14:23:39Z

CI Vulkan-ValidationLayers build queued with queue ID 376167.

arno-lunarg · 2025-02-20T14:24:52Z

The number of detected overlaps in tests will also now depends on how std::lower_bound is implemented, hopefully all implementations work the same way

ci-tester-lunarg · 2025-02-20T14:25:02Z

CI Vulkan-ValidationLayers build # 19127 running.

artem-lunarg · 2025-02-20T15:00:50Z

@arno-lunarg std::lower_bound (and other stl algorithms) has some guarantees how it works. We can rely on them. If that's something not coverd by those rules (or can't be derived from them) then we can't.
https://en.cppreference.com/w/cpp/algorithm/lower_bound (or the spec)

ci-tester-lunarg · 2025-02-20T15:00:56Z

CI Vulkan-ValidationLayers build # 19127 failed.

spencer-lunarg · 2025-02-20T15:24:51Z

tests/unit/ray_tracing.cpp

+        // elements.
+        // => due to validation code optimisations, not *all* overlaps will be detected,
+        // but if there is *at least one*, it will *always+ be detected.
+        m_errorMonitor->SetDesiredError("VUID-vkCmdBuildAccelerationStructuresKHR-dstAccelerationStructure-03703");


Suggested change

m_errorMonitor->SetDesiredError("VUID-vkCmdBuildAccelerationStructuresKHR-dstAccelerationStructure-03703");

m_errorMonitor->SetDesiredError("VUID-vkCmdBuildAccelerationStructuresKHR-dstAccelerationStructure-03703", 3);

spencer-lunarg · 2025-02-20T15:43:05Z

layers/core_checks/cc_ray_tracing.cpp

+                        break;
+                    }
+                    case AddressRangeOrigin::DstAccelStruct: {
+                        const auto other_dst_as_state =


can this be null?

possibly, will take care of this one and the others

just a ASSERT_AND_RETURN type thing should be good

spencer-lunarg · 2025-02-20T15:43:20Z

layers/core_checks/cc_ray_tracing.cpp

+                        break;
+                    }
+                    case AddressRangeOrigin::DstAccelStruct: {
+                        const auto other_dst_as_state =


same, can this be null?

spencer-lunarg · 2025-02-20T15:44:58Z

layers/core_checks/cc_ray_tracing.cpp

+
+        {
+            const rt::BuildType rt_build_type =
+                error_obj.location.function == Func::vkBuildAccelerationStructuresKHR ? rt::BuildType::Host : rt::BuildType::Device;


can this be removed, this check is only for device call now I think

yes indeed!

ci-tester-lunarg · 2025-02-20T15:57:46Z

CI Vulkan-ValidationLayers build queued with queue ID 376329.

spencer-lunarg · 2025-02-20T15:58:42Z

layers/state_tracker/state_tracker.cpp

+void Device::PreCallRecordCreateBuffer(VkDevice device, const VkBufferCreateInfo *pCreateInfo,
+                                       const VkAllocationCallbacks *pAllocator, VkBuffer *pBuffer, const RecordObject &record_obj,
+                                       chassis::CreateBuffer &chassis_state) {
+    if (pCreateInfo->usage & VK_BUFFER_USAGE_ACCELERATION_STRUCTURE_STORAGE_BIT_KHR) {


this is playing a bit with fire, and there is a chance this might cause a false positive if we have now altered what the drive reports back and the user will see a different result with and without validation

Another option/idea is if they don't have the VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT flag, we just don't check it.

I guess personally want to be on the side of "no false positive" to trying to validate everything, but at the same time, not sure if this will "save many mistakes" that otherwise would be missed if we don't enforce it

Yes it is a compromise, for now I deem it well worth it. VVL will still report an error when application forgot to set this flag, I don't expect major problems around here, but we will see.
It is impossible to not have shader device address when using acceleration structure, at least because you need them for the scratch buffer

And this would not be the first time a debug/validation tool modifies the behavior of the targeted app, just look at GPU-AV, we do things far more horrible!

yes, but this is not GPU-AV and we have had issues where pipeline creation was setting ignored fields to null and caused lots of headaches

basically people don't expect core validation to alter things, just a read only validation... again, should be fine, but will keep our eyes on the lookout for trouble

Yes I will keep this change in mind should ray tracing weirdness come

ci-tester-lunarg · 2025-02-20T15:59:04Z

CI Vulkan-ValidationLayers build queued with queue ID 376344.

ci-tester-lunarg · 2025-02-20T16:00:02Z

CI Vulkan-ValidationLayers build # 19130 running.

spencer-lunarg

LGTM - when null check comments are addressed

ci-tester-lunarg · 2025-02-20T16:10:55Z

CI Vulkan-ValidationLayers build queued with queue ID 376377.

ci-tester-lunarg · 2025-02-20T16:15:01Z

CI Vulkan-ValidationLayers build # 19132 running.

ci-tester-lunarg · 2025-02-20T16:19:21Z

CI Vulkan-ValidationLayers build queued with queue ID 376393.

ci-tester-lunarg · 2025-02-20T16:20:01Z

CI Vulkan-ValidationLayers build # 19133 running.

ci-tester-lunarg · 2025-02-20T17:06:53Z

CI Vulkan-ValidationLayers build # 19133 failed.

ci-tester-lunarg · 2025-02-20T17:09:21Z

CI Vulkan-ValidationLayers build queued with queue ID 376507.

ci-tester-lunarg · 2025-02-20T17:10:01Z

CI Vulkan-ValidationLayers build # 19138 running.

ci-tester-lunarg · 2025-02-20T17:53:08Z

CI Vulkan-ValidationLayers build # 19138 failed.

ci-tester-lunarg · 2025-02-20T20:56:13Z

CI Vulkan-ValidationLayers build queued with queue ID 376717.

ci-tester-lunarg · 2025-02-20T20:58:59Z

CI Vulkan-ValidationLayers build # 19144 running.

ci-tester-lunarg · 2025-02-20T21:25:22Z

CI Vulkan-ValidationLayers build # 19144 failed.

ci-tester-lunarg · 2025-02-21T09:07:35Z

CI Vulkan-ValidationLayers build queued with queue ID 377060.

ci-tester-lunarg · 2025-02-21T09:10:05Z

CI Vulkan-ValidationLayers build # 19151 running.

ci-tester-lunarg · 2025-02-21T09:13:27Z

CI Vulkan-ValidationLayers build queued with queue ID 377076.

ci-tester-lunarg · 2025-02-21T09:15:05Z

CI Vulkan-ValidationLayers build # 19152 running.

ci-tester-lunarg · 2025-02-21T09:51:11Z

CI Vulkan-ValidationLayers build # 19152 failed.

To see what kind of addresses drivers emit

ci-tester-lunarg · 2025-02-21T09:58:52Z

CI Vulkan-ValidationLayers build queued with queue ID 377104.

ci-tester-lunarg · 2025-02-21T10:00:01Z

CI Vulkan-ValidationLayers build # 19153 running.

ci-tester-lunarg · 2025-02-21T10:32:18Z

CI Vulkan-ValidationLayers build # 19153 failed.

arno-lunarg · 2025-02-21T10:43:56Z

After getting some info back from the working group, it seems that relying on device addresses to look for memory overlaps will not work, at least because of this:
If you have 2 sparse buffers, bound to the same memory, they can still return different base device addresses....

So I need to revamp this PR

arno-lunarg requested a review from a team as a code owner February 20, 2025 14:21

arno-lunarg force-pushed the arno-rt-validation-perf-2 branch from c4d1142 to 3856539 Compare February 20, 2025 14:23

spencer-lunarg reviewed Feb 20, 2025

View reviewed changes

arno-lunarg force-pushed the arno-rt-validation-perf-2 branch from 3856539 to 4c15ced Compare February 20, 2025 15:57

spencer-lunarg reviewed Feb 20, 2025

View reviewed changes

arno-lunarg force-pushed the arno-rt-validation-perf-2 branch from 4c15ced to e25bba1 Compare February 20, 2025 15:59

spencer-lunarg approved these changes Feb 20, 2025

View reviewed changes

arno-lunarg force-pushed the arno-rt-validation-perf-2 branch from e25bba1 to 8f40b0f Compare February 20, 2025 16:10

arno-lunarg force-pushed the arno-rt-validation-perf-2 branch from 8f40b0f to 7e5a918 Compare February 20, 2025 16:19

arno-lunarg force-pushed the arno-rt-validation-perf-2 branch from 7e5a918 to 00898b6 Compare February 20, 2025 17:09

arno-lunarg force-pushed the arno-rt-validation-perf-2 branch from a9a9c7f to f0f2b76 Compare February 21, 2025 09:07

arno-lunarg force-pushed the arno-rt-validation-perf-2 branch from f0f2b76 to ed78078 Compare February 21, 2025 09:13

arno-lunarg added 2 commits February 21, 2025 10:58

vvl: Rework accel structs mem aliasing validation

903121a

vvl: Debug logs (temporary)

d0effb9

To see what kind of addresses drivers emit

arno-lunarg force-pushed the arno-rt-validation-perf-2 branch from ed78078 to d0effb9 Compare February 21, 2025 09:58

arno-lunarg closed this Feb 21, 2025

arno-lunarg mentioned this pull request Feb 24, 2025

vvl: Rework accel structs mem aliasing validation, re-vengeance #9516

Merged

	m_errorMonitor->SetDesiredError("VUID-vkCmdBuildAccelerationStructuresKHR-dstAccelerationStructure-03703");
	m_errorMonitor->SetDesiredError("VUID-vkCmdBuildAccelerationStructuresKHR-dstAccelerationStructure-03703", 3);

vvl: Rework accel structs mem aliasing validation #9492

vvl: Rework accel structs mem aliasing validation #9492

Conversation

arno-lunarg commented Feb 20, 2025 • edited Loading

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

arno-lunarg commented Feb 20, 2025 • edited Loading

ci-tester-lunarg commented Feb 20, 2025

artem-lunarg commented Feb 20, 2025 • edited Loading

ci-tester-lunarg commented Feb 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ci-tester-lunarg commented Feb 20, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

spencer-lunarg left a comment

Choose a reason for hiding this comment

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 20, 2025

ci-tester-lunarg commented Feb 21, 2025

ci-tester-lunarg commented Feb 21, 2025

ci-tester-lunarg commented Feb 21, 2025

ci-tester-lunarg commented Feb 21, 2025

ci-tester-lunarg commented Feb 21, 2025

ci-tester-lunarg commented Feb 21, 2025

ci-tester-lunarg commented Feb 21, 2025

ci-tester-lunarg commented Feb 21, 2025

arno-lunarg commented Feb 21, 2025

arno-lunarg commented Feb 20, 2025 •

edited

Loading

arno-lunarg commented Feb 20, 2025 •

edited

Loading

artem-lunarg commented Feb 20, 2025 •

edited

Loading