Re: [PATCH 0/4] MR766: Add more stubbed cache infrastructure

[Henri Verbeet (＠hverbeet)]

> +    if (!info)
> +    {
> +        WARN("No cache info, returning VKD3D_ERROR_INVALID_ARGUMENT.\n");
> +        return VKD3D_ERROR_INVALID_ARGUMENT;
> +    }
> +    if (info->filename && !(size = strlen(info->filename)))
> +    {
> +        WARN("Filename is an empty string, returning VKD3D_ERROR_INVALID_ARGUMENT.\n");
> +        return VKD3D_ERROR_INVALID_ARGUMENT;
> +    }

Do we need to explicitly verify these? It feels a little superfluous.
> +unsigned int vkd3d_shader_cache_incref(struct vkd3d_shader_cache *cache)
> +{
> +    unsigned int refcount = vkd3d_atomic_increment_u32(&cache->refcount);
> +    TRACE("cache %p refcount %u.\n", cache, refcount);
> +    return refcount;
> +}
> +
> +unsigned int vkd3d_shader_cache_decref(struct vkd3d_shader_cache *cache)
> +{
> +    unsigned int refcount = vkd3d_atomic_decrement_u32(&cache->refcount);
> +    TRACE("cache %p refcount %u.\n", cache, refcount);
> +
> +    if (refcount)
> +        return refcount;
> +
> +    vkd3d_free(cache);
> +    return 0;
> +}

Do we need refcounts? I may be misremembering, but I think we were moving towards only opening caches once, and the caller then being responsible for managing their lifetime, at least on the vkd3d-shader level. In any case, this is unused code in this commit.
> +enum vkd3d_shader_cache_flags
> +{
> +    VKD3D_SHADER_CACHE_FLAGS_NONE =             0x00000000,
> +    VKD3D_SHADER_CACHE_FLAGS_NO_SERIALIZE =     0x00000001,
> +    VKD3D_SHADER_CACHE_FLAGS_READ_ONLY =        0x00000002,
> +
> +    VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_CACHE_FLAGS),
> +};
> +
> +struct vkd3d_shader_cache_info
> +{
> +    /*enum vkd3d_structure_type type;*/
> +    const void *next;
> +
> +    /** File name to open, or NULL for a memory-only cache. */
> +    const char *filename;
> +    /** Maximum amount of data the cache holds in memory. */
> +    uint64_t mem_size;
> +    /** Maximum amount of data written to disk. Ignored if filename is NULL. */
> +    uint64_t disk_size;
> +    /** Maximum number of cache entries. */
> +    uint64_t max_entries;
> +    /** Random flags, what else. */
> +    enum vkd3d_shader_cache_flags flags;
> +    /** An application-chosen version number. If the version of an existing
> +     * cache on disk does not match, the old data will be discarded. */
> +    uint64_t version;
> +};

Somewhat similarly, much of this is unused in this MR.
> I don't have objections against merging this into the previous patch.

That would help, but note that you'd still have a decent number of unused things.
> +    /* We expect the number of open caches to be small. */
> +    LIST_FOR_EACH_ENTRY(i, &cache_list, struct d3d12_cache_session, cache_list_entry)
> +    {
> +        if (!memcmp(&i->desc.Identifier, &desc->Identifier, sizeof(desc->Identifier)))
> +        {
> +            TRACE("Found an existing cache %p from session %p.\n", i->cache, i);
> +            if (desc->Version == i->desc.Version)
> +            {
> +                session->desc = i->desc;
> +                vkd3d_shader_cache_incref(session->cache = i->cache);
> +                break;
> +            }
> +            else
> +            {
> +                WARN("version mismatch: Existing %"PRIu64" new %"PRIu64".\n",
> +                        i->desc.Version, desc->Version);
> +                hr = DXGI_ERROR_ALREADY_EXISTS;
> +                goto error;
> +            }
> +        }
> +    }

Should we just return the existing ID3D12ShaderCacheSession?