Open python3kgae opened 1 year ago
This proposal (mostly) talks about diagnosing local errors until it gets to step 5, which is the step corresponding to where DXC would assign bindings and report conflicts only to the remaining used resources in DxilResourceRegisterAllocator::AllocateRegisters
. The non-local overlapping binding diagnostics from step 3 is also handled at this point in DXC.
The reason this happens late is that unused resources should be removed by this point and not be allocated any binding location, and explicit bindings can only conflict between resources that are actually used in the same compiled entry point.
The suggestions here will require a change to what's considered a "used" resource. One possibility is to change the definition to one based on reachability. Another is to just decide that all declared resources will be bound and must not conflict regardless of usage.
One of these changes might be acceptable, but it would need to be explicitly specified and noted as one of the more impactful breaking changes when moving from DXC to Clang.
This is also a change to DXIL library behavior, since by default a library preserves the unbound state of a resource so the used set of unbound resources may be assigned once a particular entry point is linked to a final shader. This is the reason for the -auto-binding-space
option, which causes automatic binding to occur on the library in the selected space.
Note that I did explicitly call this a strawman ;)
Yes, if we were to do this in Sema we definitely have to make some decisions on how to define a "used" resource. I think a reachability based approach is somewhat tenable if we're allocating the resources during ActOnEndOfTranslationUnit
- we can track which bindings have been "seen" as we go through Sema and filter them out at this point. This is where diagnosis of things like unused private fields happens so it might be feasible, but we'd need to look into it some more.
As for library behaviour, having explicitly unbound resources is probably necessary. We'll probably want to keep track of whether bindings were user provided or compiler generated anyway.
If on the other hand we really want to stick to the "if it happens to be optimized out, then it's unused" model, then this is a lot less practical indeed.
If I ruled the world, I would require explicit bindings on all resources. I'm having a quick look to see if I can easily look at my collection of shaders to see how common it is for resources not to have an explicit binding.
edit: that was quick. The first one I opened was full of implicitly bound textures.
When a global resource doesn't have a register binding attribute, we need to allocate a binding for the it.
We should try to share code between the register binding allocation and the diagnostics for invalid bindings. A strawman design for sharing the code would probably look something like this:
SemaHLSL::handleResourceBindingAttr
just diagnoses the simple/local errors:err_hlsl_binding_type_invalid
- the type isn't valid at all, likex0
warn_hlsl_deprecated_register_type_i
- the type is deprecated/removed and ignoredwarn_hlsl_deprecated_register_type_b
- the type is deprecated/removed and ignoredwarn_hlsl_register_type_c_packoffset
- the type is in the wrong place (in acbuffer
)warn_hlsl_overlapping_binding
- the same binding type is applied more than onceThese shouldn't need any traversal of types to diagnose, so keeping them here is simplest.
Add a data member,
Bindings
, toSemaHLSL
to track the ranges that are already bound and the list ofDecl
s that still need to be bound.In
Sema::ActOnVariableDeclarator
(inSemaDecl.cpp
) after the call toProcessDeclAttributes
, call intoSemaHLSL
:This would recursively walk the type similarly to the traversal described in this proposal, but instead of setting flags we'd calculate the size of the UAV, SRV, CBuffer, and Sampler bindings to add to
SemaHLSL::Bindings
.If a binding has zero size but we have an attribute for the binding we diagnose a mismatch:
warn_hlsl_binding_type_mismatch
if it's a UDTerr_hlsl_binding_type_mismatch
if it's a simple typeThis is where we would diagnose a new
warn_hlsl_overlapping_binding
if multiple separate things are bound in overlapping places.In
SemaHLSL::ActOnFinishBuffer
, add the binding for the cbuffer/tbuffer as a whole, using the size we calculate for diagnostics already. This will also need to diagnose a couple of things:err_hlsl_binding_type_mismatch
if it's a simple typewarn_hlsl_overlapping_binding
Note that doing this here simplifies the parts of the design that need to differentiate between
HLSLBufferDecl
andVarDecl
, since the two code paths are separate.Later on in Sema (
ActOnEndOfTranslationUnit
?) we walk the list of unboundDecl
s and assign them bindings.