video_core: Account of runtime state changes when compiling shaders #575
Conversation
|
This looks like a promising PR since as far as I can tell BB crashes due to device loss on main branch because some shader tries to access an uniform buffer with a size greater than 64k (the limit on most NVIDIA GPUs) However, the game now crashes on startup due to this assertion. I suppose this shader could be skipped but I have no clue how to do that in the new architecture inside pipeline_cache |
|
Yes this PR has exposed some preexisting shader issues in BB that need to be solved (mainly writing to cubemaps with imageStore and more buffer store formats) |
|
With latest commits Bloodborne reaches character creator but immediately crashes with this error |
Can confirm, I also reproduced it by going into the loading menu. Instantly crashes, probably when trying to render character models. ... |
There was a problem hiding this comment.
Edit: this part in vk_pipeline_cache.h:GetProgram, I messed up something in the comment
const auto it = std::ranges::find(program->modules, stage_key, &Program::Module::first);
if (it == program->modules.end()) {
auto new_info = MakeShaderInfo(stage, pgm->user_data, info.pgm_base, info.pgm_hash,
liverpool->regs);
const size_t perm_idx = program->modules.size();
const auto module = CompileModule(new_info, pgm->Code(), perm_idx, binding);
program->modules.emplace_back(stage_key, module);
} else {
binding += info.NumBindings();
}
const u64 full_hash = HashCombine(hash, stage_key);
return std::make_tuple(&info, it->second, full_hash);I can see how program->modules.end() points to the freshly emplaced element so you just reuse the iterator when returning from this function.
Some shaders cause the small_vector to allocate on the heap, invalidating this iterator and propagating VK_NULL_HANDLE to the pipeline creation functions.
|
Good catch, yes. The shader module needs to be copied and not use iterator |
|
For the bloodborne asserts we need to switch to imageBuffer to get automatic format conversion on buffer store. This should reduce permutations in the generic cs clear shaders that trigger most often. Also we need a descriptor set per shader stage ideally to avoid needing to hash start binding |
raphaelthegreat
force-pushed
the
permut
branch
2 times, most recently
from
77f10ce to
049a3df
Compare
raphaelthegreat
force-pushed
the
permut
branch
from
049a3df to
e96a6eb
Compare
|
For now Bloodborne [CUSA03173] still crashes when making a new character. Game is completely unmodded for testing. [Render.Vulkan] vk_shader_cache.cpp:CompileModule:116: Compiling fs shader 0x5e224cb6 |
|
Seems to be corrupted on intel gpus |
A lot of state used during shader compilation is stored in the resource definitions (sharps). For buffers the stride, format (in case of untyped buffer load/store) as well as the size are relevant to compilation. For images the type is most relevant as that determines the layout of the address stream.
Due to this, it is possible for these parameters to be modified while the same shader program is bound. This can cause a variety of issues which are difficult to debug. For example a read-only buffer may get compiled as a UBO first time, but then a larger buffer may be bound which cannot fit in the 16KB size limit. Some games can also use multiple image types with the same program.
To solve this, each Program (which represents a unique guest shader) can now store a collection of vulkan shader modules. After its fetched the resource layout is determined and a resource stage key is calculated which takes into account the position of the shader in the pipeline (start binding) as well as resource information relevant to compilation. Might fix issues in some games, but can also expose issues that where hidden before. This will allow us to remove another hack in bb-hacks branch but is not a fix for bloodborne specifically.
In addition memory usage from the shader cache should be significantly reduced. We only cache Shader::Info object per shader as well as the vulkan shader modules. Before this change, the generated SPIRV code was kept around which could be dozens of KBs per object as well as the IR::Program which stored blocks and translated gcn instructions that aren't much useful