<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, Dec 29, 2015 at 1:56 PM, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Dec 29, 2015 at 4:52 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > > > On Tue, Dec 29, 2015 at 1:50 PM, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> wrote: >> >> On Tue, Dec 29, 2015 at 4:45 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> >> wrote: >> > >> > >> > On Mon, Dec 28, 2015 at 11:06 AM, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> >> > wrote: >> >> >> >> Currently any access params (coherent/volatile/restrict) are being lost >> >> when lowering to the ssbo load/store intrinsics. Keep track of the >> >> variable being used, and bake its access params in as the last arg of >> >> the load/store intrinsics. >> >> >> >> Signed-off-by: Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> >> >> --- >> >> >> >> This is RFC because there are still no users, but I'm working on that. >> >> Don't >> >> want to be going in the completely wrong direction though... >> >> >> >> src/glsl/lower_ubo_reference.cpp | 22 ++++++++++++++++++++++ >> >> 1 file changed, 22 insertions(+) >> >> >> >> diff --git a/src/glsl/lower_ubo_reference.cpp >> >> b/src/glsl/lower_ubo_reference.cpp >> >> index a172054..5b3f523 100644 >> >> --- a/src/glsl/lower_ubo_reference.cpp >> >> +++ b/src/glsl/lower_ubo_reference.cpp >> >> @@ -104,6 +104,7 @@ public: >> >> >> >> struct gl_shader *shader; >> >> struct gl_uniform_buffer_variable *ubo_var; >> >> + ir_variable *variable; >> >> ir_rvalue *uniform_block; >> >> bool progress; >> >> }; >> >> @@ -317,6 +318,7 @@ >> >> lower_ubo_reference_visitor::handle_rvalue(ir_rvalue >> >> **rvalue) >> >> this->buffer_access_type = >> >> var->is_in_shader_storage_block() ? >> >> ssbo_load_access : ubo_load_access; >> >> + this->variable = var; >> >> >> >> /* Compute the offset to the start if the dereference as well as >> >> other >> >> * information we need to configure the write >> >> @@ -370,6 +372,13 @@ shader_storage_buffer_object(const >> >> _mesa_glsl_parse_state *state) >> >> return state->ARB_shader_storage_buffer_object_enable; >> >> } >> >> >> >> +static uint32_t ssbo_access_params(const ir_variable *var) >> >> +{ >> >> + return (var->data.image_coherent << 0) | >> >> + (var->data.image_volatile << 1) | >> >> + (var->data.image_restrict << 2); >> > >> > >> > Do 0, 1, and 2 have some special meaning I should know about? I suggest >> > we >> > add an enum to shader_enums.h. >> >> Do you mean a struct with a bitfield? That'd work for me. > > Well, if you want to pack it into a function argument you intend to bake > into the shader source, that won't work so well. It needs to be an integer > in the end. If you want to do a union and a bitfield, that's fine, but I > think an enum is more customary. </div></div>Er right... I want an integer in the end. But I want all 3 values to be packed into it... I could add defines for 1, 2, and 4, like SSBO_COHERENT, SSBO_VOLATILE, SSBO_RESTRICT. An enum seems wrong since it's a bitfield with multiple possible bits set, not an enumeration. </blockquote><div> </div><div>Right. Although gdb does nice things for you if you use an enum with values 1, 2, and 4. That said, it seems like what we want is some "qualifiers" bitfield enum and then just get rid of the var->data.image_foo things. </div><div>--Jason </div></div> </div></div>