Add more stubbed cache infrastructure
This adds the beginnings of vkd3d_shader_cache and links it up with ID3D12ShaderCacheSession. No actual data is stored yet, this will come in the next series.
Merge request reports
Activity
+ 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?
Do we need to explicitly verify these? It feels a little superfluous.
As long as these functions are private in libvkd3d we don't necessarily need them, but once this becomes public I want to catch parameters that would cause problems - e.g. a zero-length file name makes a cache memory only.
I can remove them for now and add them right before or in the same commit that moves this code to vkd3d_shader.
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.
CreateShaderCacheSession returns different objects when reopening an existing cache. This is observable not only by comparing pointers, but also e.g. through SetPrivateData et al. I thought I already had tests for that in the test submitted in the previous MR, but I see they are missing there because they are mixed with the StoreValue/FindValue tests. I'll add some to this MR.
Do we need to explicitly verify these? It feels a little superfluous.
As long as these functions are private in libvkd3d we don't necessarily need them, but once this becomes public I want to catch parameters that would cause problems - e.g. a zero-length file name makes a cache memory only.
Should it? I think we could just require info->filename to be NULL for that case.
I can remove them for now and add them right before or in the same commit that moves this code to vkd3d_shader.
That's fine in any case.
Should it? I think we could just require info->filename to be NULL for that case.
My comment was phrased poorly - from the caller's side yes, info->filename = NULL will be the way to request a memory only cache.
What I wanted to say is that in the case of a memory only cache cache->filename (dynamic length array at the end of struct vkd3d_shader_cache) becomes an empty string and I want to prevent cases where the caller passes info->filename = "". Although in practise I'd expect fopen("") to fail and such a cache never be created with or without an explicit check.
added 31 commits
-
9a0da63b...d1e16514 - 28 commits from branch
wine:master
- 65dbe689 - tests: Add CreateShaderCacheSession pointer clearing tests.
- 4d1489bb - vkd3d: Implement opening and closing shader caches.
- 67ff7e5a - vkd3d: Implement reopening existing caches.
Toggle commit list-
9a0da63b...d1e16514 - 28 commits from branch
added 22 commits
-
67ff7e5a...42f07352 - 19 commits from branch
wine:master
- 26387e1d - tests: Add CreateShaderCacheSession pointer clearing tests.
- f2400550 - vkd3d: Implement opening and closing shader caches.
- a7860ae7 - vkd3d: Implement reopening existing caches.
Toggle commit list-
67ff7e5a...42f07352 - 19 commits from branch