italove has quit [Remote host closed the connection]
italove has joined #zink
i509vcb has quit [Quit: Connection closed for inactivity]
hikiko has quit []
pac85 has joined #zink
<pac85>
Uver shader compilation is something I've done some time ago before our discussions, I basically added emulation_{obj, gpl} to zink_shader::precompile. I populate those the same way regular obj and gpl fields are populated (so the job is started at cso creation). Now I suppose this won't do? Do mentioned that uber shaders should be built sort of on demand, so I guess their compilation should be kicked off in update_gfx_program?
<zmike>
what do you mean "on demand"
<pac85>
When we need a variant for the first time
<zmike>
to be clear, are we talking about mesa/st legacy variants, the current zink variants, or the "new" variants
<pac85>
both st and zink variant, I don't treat them that differently
<zmike>
hm
<zmike>
so in that case I think the precompile job should just include all variants since it's an ubershader?
<zmike>
so the precompiled shader will be big and include everything, but there will only be one
<pac85>
Yes, that was the idea right?
<zmike>
yeah
<pac85>
My question is when do I start compiling it
<pac85>
right now I do it as soon as I can (cso creation)
<zmike>
I think it depends
<zmike>
basically any time precompile_separate_shader_job or precompile_job are called
<zmike>
separate_shader is from cso creation
<zmike>
the other case is from zink_link_gfx_shader
<pac85>
It's kind of what I'm doing rn so great
<zmike>
yeah basically this new thing should replace the existing model
<zmike>
is how I'd imagine it working
<pac85>
Ah, so should I get rid of the precompiled base shader? I left it there in the case where no emulation is needed but thinking about it the uber shader can obviosuly handle that
<zmike>
what do you mean "precompiled base"
<pac85>
what zink precompiles right now
<zmike>
zink_shader::precompile is for separate shaders
<zmike>
so that can't be eliminated since it's different
<pac85>
I was compiling the uber shaders as separate as well actually
<zmike>
can't do that
<zmike>
separate shaders have explicit restrictions on i/o which ensures the variables will match up
<zmike>
non-sso doesn't have those restrictions, so the shaders can't be compiled separately
<pac85>
Mmm I see
<pac85>
And how do we compile when separate shaders are used?
<zmike>
it goes through precompile_separate_shader_job and create_gfx_program_separable
<pac85>
I mean, how do I compile the uber shader when that path is taken? Currenty I stuffed the uber shader stuff in that path
<zmike>
the uber (nir) would get created in precompile_separate_shader_job
<pac85>
Just the nir? without creating a gpl with the separate shaders? and then where would I create the pipelines?
<zmike>
precompile_separate_shader_job calls zink_create_gfx_pipeline_separate if shader object isn't supported
<zmike>
I'd expect zink_shader_compile_separate to run whatever nir passes for ubershaders and then the rest of the code to work like normal
<pac85>
Yes this is what I'm doing. I though you said it wasn't ok
<zmike>
I don't think so?
<zmike>
or if I did I was wrong
<pac85>
I probably misintepreted
<zmike>
the only thing to be done for ubershaders is run some nir passes and update push constants, yes?
<pac85>
Yeah
<zmike>
so the nir pass running can just be done in the places I described above
<zmike>
and that should be the only changes required for precompile
<pac85>
Oh so the uber shaders should be used indefinitely when legacy features are used? I was working on asyncrhounsly precompiling shaders using the regular lowering passes with the key and then replacing the uber with those
<zmike>
I'm only talking about precompile
<zmike>
optimized variants should be background compiled
<pac85>
great
<zmike>
it sounds like you're on top of it
<pac85>
Now I faced a compilcation
<pac85>
when shaders are precompiled I end up with a prog which has is_separate set to true
<zmike>
isn't that just because you're doing separate shader precompile for all shaders?
<pac85>
No I mean when separate shaders are used by the gallium frontend
<pac85>
Now in order to update the modules on a prog I need a full prog
<zmike>
ah
<pac85>
and currently zink waits for that to be available when vasriants are used
<zmike>
and?
<pac85>
Well the problem is that if I let zink replace the separable prog then then uber shader is lost, and I can't start precompiling the optimized variants because I can't update the modules on a separable prog
<zmike>
hm
<zmike>
might need to modify the separate shader handling
<pac85>
That's what I feared
<zmike>
i.e., keep the is_separate prog and then do "variant prog" lookups
<zmike>
maybe with a "base variant" (default key values) directly attached with no lookup
<pac85>
mmm sounds interesting, so I would add a dynarray of progs to prog right?
<zmike>
maybe just hash table to start, but yeah
<pac85>
Off to implementing that, thanks! Also non code-related question. Is it ok if we don't support every single lowering pass for the uber shader and fall back to drawtime compile for those we don't support? At least initially. It would make it easier to land the patch because it means I can land the logic and a few passes at first then worry about cleaning up the other passes later (those don't conflict as much when rebasing)
<zmike>
yeah I think it's okay to do it incrementally
<zmike>
the branchpoint is in ~2 weeks, so I think if we start landing it after that then we can just be sure to land it all before the branchpoint after that and all good
<pac85>
Sounds great!
<zmike>
cool
<zmike>
looking forward to seeing the MR
<pac85>
I want to get the logic right (in the sense that nothing explodes) then I guess I can clean up the commits just for that and open a draft so I can get more feedback. Then I have 70 commits to go through