Pipeline Layout Compatibility state tracking for descriptors should not compare push constant range sizes

[scygan]

This happened for me during development of not yet public product, so no steams / platform details are provided.

Describe the Issue

I am getting what seems to me a false positive:

VUID-vkCmdDispatch-None-08600(ERROR / SPEC): ... | vkCmdDispatch():  VkPipeline ... uses set #0 but that set is not bound...

In following scenario:

There are two pipelines used, A and B, used for subsequent compute dispatches. These pipelines use different pipeline layouts. However, both pipeline layouts are 'compatible for set 0', meaning the first descriptor set layout in each pipeline layout is identically defined (though it is not the same object).

Moreover, what seems important to this issue, the push constant range sizes are different in each pipeline layout.

- vkCmdBindPipeline(A)
- vkCmdBindDescriptorSet(firstSet=0, pipelineLayout=A)
- vkCmdBindDescriptorSet(firstSet=1, pipelineLayout=A)
- vkCmdBindDescriptorSet(firstSet=2, pipelineLayout=A)
- vkCmdDispatch()
- vkCmdBindPipeline(B)
- vkCmdBindDescriptorSet(firstSet=1, pipelineLayout=B)
- vkCmdBindDescriptorSet(firstSet=2, pipelineLayout=B)
- vkCmdBindDescriptorSet(firstSet=2, pipelineLayout=B)
- vkCmdDispatch()   // <<<<<<< VUID emitted here

It looks like during the vkCmdBindDescriptorSet(firstSet=1, pipelineLayout=B) call, following code resets the binding for set 0, bacause set_info.compat_id_for_set != pipe_compat_ids[set_idx] evaluates to true for set_index equal to 0:

    // For any previously bound sets, need to set them to "invalid" if they were disturbed by this update
    for (uint32_t set_idx = 0; set_idx < first_set; ++set_idx) {
        auto &set_info = last_bound.per_set[set_idx];
        if (set_info.compat_id_for_set != pipe_compat_ids[set_idx]) {
            PushDescriptorCleanup(last_bound, set_idx);
            set_info.Reset();
            set_info.compat_id_for_set = pipe_compat_ids[set_idx];
        }
    }

The problem is both set_info.compat_id_for_set and pipe_compat_ids[set_idx] are hashed using a function, that includes push constant ranges in the hash, so the hashes (and compat_id, implied by hashes) are different if push constant range size differs:

size_t PipelineLayoutCompatDef::hash() const {
    hash_util::HashCombiner hc;
    // The set number is integral to the CompatDef's distinctiveness
    hc << set << push_constant_ranges.get();
    const auto &descriptor_set_layouts = *set_layouts_id.get();
    for (uint32_t i = 0; i <= set; i++) {
        hc << descriptor_set_layouts[i].get();
    }
    return hc.Value();
}

As a debugging measure, I have tried using same push constant range size for all pipeline layouts and the problem went away.

Expected behavior

Spec says:

When binding a descriptor set (see Descriptor Set Binding) to set number N, a previously bound descriptor set bound with lower index M than N is disturbed if the pipeline layouts for set M and N are not compatible for set M. Otherwise, the bound descriptor set in M is not disturbed. If, additionally, the previously bound descriptor set for set N was bound using a pipeline layout not compatible for set N, then all bindings in sets numbered greater than N are disturbed. When binding a pipeline, the pipeline can correctly access any previously bound descriptor set N if it was bound with compatible pipeline layout for set N, and it was not disturbed.

Especially: it seems to me the spec does not mention that "compatiblity for set" requires pipeline layouts to be “compatible for push constants” ...

Valid Usage ID

VUID-vkCmdDispatch-None-08600(

[spencer-lunarg]

So there is a lot to unpack

• Your Pipeline Layouts are not compatible if the push constants ranges are not identical
• I know if you call vkCmdPushConstants with the non-compatible layout it is legal (but value is TBD)
• This seems like it could be invalid, but definitely not for VUID-vkCmdDispatch-None-08600

I think my goal is to first write up a test to mimic this, then make sure it is not falsely reporting 08600 and figure out with the Working Group if there is suppose to be any VU related to 08600 for this case

[scygan]

@spencer-lunarg, yes I would call vkCmdPushConstants, I have removed this from above list of called functions.

However, having different push constant size should not invalidate bound descriptor sets. Only incompatible descriptor set layouts should invalidate descriptor sets.

[scygan]

Pinging @spencer-lunarg.

Is there any more info needed to fix this issue?

[spencer-lunarg]

info, no... the blocking issue here is just time to spend looking into this and writing tests, sorry it has taken so long to get to this