implemented indirect dispatch #66

KirillAlekseeenko · 2024-07-01T20:05:13Z

implemented indirect dispatch

Try

Need a unit test for new api:
gapi_test_common.h is a template for api-tests.
resourcestate_test.cpp is also important for sync testing.

Currently there is api-tests for indirect, so it's fine to skip those, but sync test is must-have, given how complex resource tracking is here.

Try · 2024-07-01T20:59:07Z

Engine/gapi/vulkan/vcommandbuffer.cpp

+
+  curUniforms->ssboBarriers(resState, PipelineStage::S_Compute);
+  // block future writers
+  resState.onUavUsage(ind.nonUniqId, NonUniqResId::I_None, PipelineStage::S_Compute);


This should not be enough, for vulkan.

Basically you need a RaW barrier [WR] -> Indirect, where WR - any stage that might been writing into this buffer. And then you need to mark this buffer as in read-state (S_Compute mean the shader, not indirect command).

I'm assuming you looked into mesh-shader code, as example. Draw-calls are a bit hacky today, as they assume full barrier between compute and graphics, so no need to emit RaW.

Do you mean that there might be the case that after indirect dispatch
resState.onUavUsage(NonUniqResId::I_None, %indirectBufferId%, %anyStage%) won't be called before any write access to the indirect buffer?

I mean, resState wont be enough today to produce strong enough barrier.

Code that you added, resState.onUavUsage(ind.nonUniqId, NonUniqResId::I_None, PipelineStage::S_Compute) will mark buffer as 'read', for compute stage. If you have had another compute job before that wrote to indirect-buffer, then it wall cause only CS -> CS barrier, but you need a stronger one: CS -> Indirect.

For vulkan only correct solution, would be to introduce indirect as a stage, such as PipelineStage::S_Indirect.
For DX, unfortunately same idea wont work at all, as it requires precise tracking of indirect-buffers, and has to use solution similar to one used for render-targets.

This is why CS-indirect was long delayed :)

Engine/gapi/directx12/dxcommandbuffer.cpp

Engine/graphics/encoder.h

-added tests for indirect barriers -added explicit state transition to and from indirect on dx12 -added indirect pipeline stage in ResourceState for placing stronger barriers on vulkan when using indirect

redundant spaces

Try

Given how problematic DX12 is, should we merge PR without DX support (just throw exception in dispacthIndirect)?

This will unblock you on MLAA work, and in mean time I'll take care of DX12.

PS

[x] eb74d55

It was my bad - master is fixed now.

Try · 2024-07-04T16:20:03Z

Engine/gapi/directx12/dxcommandbuffer.cpp

+  resState.onUavUsage(ind.nonUniqId, NonUniqResId::I_None, PipelineStage::S_Indirect);
+  resState.flush(*this);
+
+  issueExplicitCommonToIndirectStateTransition(ind.impl.get());


There are 2 issues with this code:

you do both explicit synchronization and implicit (via onUavUsage)

resource barrier is quite expensive, and calling it per-draw is not a good call

Tests/tests/resourcestate_test.cpp

Try · 2024-07-04T16:35:22Z

Engine/gapi/vulkan/vcommandbuffer.cpp

+  curUniforms->ssboBarriers(resState, PipelineStage::S_Compute);
+  // block future writers
+  resState.onUavUsage(ind.nonUniqId, NonUniqResId::I_None, PipelineStage::S_Indirect);
+  resState.flush(*this);


What barrier will be emitted, if buffer is in used by both shader and indirect argument?
Should be SRC -> CS|indirect, plus buffer has to be in read-state.

Created test that simulates this situation:
barrier {UavReadComp | UavWriteComp -> UavReadComp | UavWriteComp | Indirect}

Propably stupid question :) but does read-state here refer to the condition that previous memory accesses for the buffer are available and visible for reading (i.e. that required barrier exists)? My first thought was about some internal state in the engine but couldn't find it.

I mean ResourceState::uavRead. Need to make sure that subsequent command are aware that indirect-buffer is in use.
What I'm worry about, that resState.flush might reset tracking preemptively.

Basically in scenario

dispath( write to buf ) --- barrier {UavReadComp | UavWriteComp -> UavReadComp | UavWriteComp | Indirect} dispathIndirect(buf) --- should be execution barrier indirect -> CS dispath( write to buf )

added subsequent write access to the test, works fine:
barrier {TransferSrc | UavReadComp | UavReadGr | RtAsRead | Indirect -> UavWriteComp}
barrier {UavReadComp | UavWriteComp -> UavReadComp | UavWriteComp | Indirect}
barrier {UavReadComp | Indirect -> UavWriteComp}

Despite clearing uavSrcBarrier and uavDstBarrier in flush(), there is a dependency on previous reads in uavRead which was written in the end of onUavUsage(read, none, compute | indirect) calls. uavRead and uavWrite are cleared only for dependencies that were added to uavSrcBarrier and uavDstBarrier

-added notImplemeted runtime exception in indirectDispatch on dx12 -added indirect resource access in barrier test output -adeed test for a situation when a buffer is binded as a UAV and Indirect simultaneously

Try · 2024-07-04T23:23:22Z

Engine/gapi/metal/mtcommandbuffer.cpp

@@ -360,6 +360,11 @@ void MtCommandBuffer::dispatch(size_t x, size_t y, size_t z) {
  encComp->dispatchThreadgroups(MTL::Size(x,y,z), localSize);
  }

+void MtCommandBuffer::dispatchIndirect(const AbstractGraphicsApi::Buffer& indirect, size_t offset) {
+  auto& ind = reinterpret_cast<const MtBuffer&>(indirect);
+  encDraw->dispatchThreadgroups(ind.impl.get(), offset, localSize);


-build fix on metal -expanded ResourceStateIndirectAndUAV test by adding a subsequent write access

Try · 2024-07-06T21:48:37Z

Merged, thanks!

implemented indirect dispatch

bcdef6e

implemented indirect dispatch

Try reviewed Jul 1, 2024

View reviewed changes

KirillAlekseeenko added 5 commits July 3, 2024 23:57

work on review comments

8d9f88e

-added tests for indirect barriers -added explicit state transition to and from indirect on dx12 -added indirect pipeline stage in ResourceState for placing stronger barriers on vulkan when using indirect

Merge branch 'Try:master' into master

0ad586c

redundant spaces

7a49739

redundant spaces

clean .gitignore from local files

ef83032

added actual usage of indirect stage

eb74d55

Try reviewed Jul 4, 2024

View reviewed changes

KirillAlekseeenko added 2 commits July 4, 2024 22:32

Merge branch 'Try:master' into master

8fdbfee

work on review comments

f5d3ce3

-added notImplemeted runtime exception in indirectDispatch on dx12 -added indirect resource access in barrier test output -adeed test for a situation when a buffer is binded as a UAV and Indirect simultaneously

Try reviewed Jul 4, 2024

View reviewed changes

build fix

cbaeeb4

-build fix on metal -expanded ResourceStateIndirectAndUAV test by adding a subsequent write access

Try merged commit 35ebfc4 into Try:master Jul 6, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implemented indirect dispatch #66

implemented indirect dispatch #66

KirillAlekseeenko commented Jul 1, 2024

Try left a comment

Try Jul 1, 2024

KirillAlekseeenko Jul 3, 2024

Try Jul 3, 2024

Try left a comment •

edited

Loading

Try Jul 4, 2024

Try Jul 4, 2024

KirillAlekseeenko Jul 4, 2024

Try Jul 4, 2024

KirillAlekseeenko Jul 5, 2024

Try Jul 4, 2024

Try commented Jul 6, 2024

implemented indirect dispatch #66

implemented indirect dispatch #66

Conversation

KirillAlekseeenko commented Jul 1, 2024

Try left a 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

Try left a comment • edited Loading

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

Try commented Jul 6, 2024

Try left a comment •

edited

Loading