Incorrect best practices message BestPractices-ImageBarrierAccessLayout

[ShabbyX]

RAR and WAR hazards don't require a memory barrier (https://github.com/KhronosGroup/Vulkan-Docs/wiki/Synchronization-Examples#first-dispatch-reads-from-a-storage-buffer-second-dispatch-writes-to-that-storage-buffer)

So if the image layout is e.g. VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, it's perfectly fine for srcAccessMask to be 0. Best practices could warn about this for dstAccessMask, but shouldn't for srcAccessMask.

Also, there is syncval, what's the point of this?

[jeremyg-lunarg]

These messages came from an issue raised by @Tobski https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/2918, which has the best (and really, only) explanation of why these warnings exist. I would much prefer that the table in the issue was in the spec somewhere, rather than in a closed VVL issue.

[Tobski]

@jeremyg-lunarg Can you raise a spec issue for this? I think I can see how we'd fit it in.

As for @ShabbyX's initial issue, #2918 only suggested warning on additional stages for roughly this reason - if the best practices layer is warning that none of the expected stages are present that seems wrong and should be left to the sync val layer.

[Tobski]

Yea looking at the original code added in #3970, it seems that the first branch of this conditional check should be removed:

    if (access == 0 && !none_allowed) {
        skip |= LogWarning(device, kVUID_BestPractices_ImageBarrierAccessLayout,
                           "%s: accessMask is VK_ACCESS_NONE, but for layout %s expected accessMask are %s.", api_name.c_str(),
                           string_VkImageLayout(layout), string_VkAccessFlags2(allowed).c_str());
    } else if ((allowed | access) != allowed) {
        skip |=
            LogWarning(device, kVUID_BestPractices_ImageBarrierAccessLayout,
                       "%s: accessMask is %s, but for layout %s expected accessMask are %s.", string_VkAccessFlags2(access).c_str(),
                       api_name.c_str(), string_VkImageLayout(layout), string_VkAccessFlags2(allowed).c_str());
    }

[jeremyg-lunarg]

Internal issue filed, you are both cc'd. I'll take a look at the logic again, and probably make a PR to fix it.

[jeremyg-lunarg]

Yea looking at the original code added in #3970, it seems that the first branch of this conditional check should be removed:

    if (access == 0 && !none_allowed) {

It isn't quite that simple because the table says GENERAL cannot have an access mask of 0. Please review the PR and make sure the new code matches your expectations.

[Tobski]

It isn't quite that simple because the table says GENERAL cannot have an access mask of 0. Please review the PR and make sure the new code matches your expectations.

Yea I don't know why I said that in hindsight. That's patently false, it should be allowed to be zero.