[Mesa-dev] [RFC PATCH] glsl: keep track of ssbo variable being accessed, add access params

[Jason Ekstrand]

On Tue, Dec 29, 2015 at 1:56 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:

> On Tue, Dec 29, 2015 at 4:52 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> >
> > On Tue, Dec 29, 2015 at 1:50 PM, Ilia Mirkin <imirkin at alum.mit.edu>
> wrote:
> >>
> >> On Tue, Dec 29, 2015 at 4:45 PM, Jason Ekstrand <jason at jlekstrand.net>
> >> wrote:
> >> >
> >> >
> >> > On Mon, Dec 28, 2015 at 11:06 AM, Ilia Mirkin <imirkin at alum.mit.edu>
> >> > 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 <imirkin at alum.mit.edu>
> >> >> ---
> >> >>
> >> >> 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.
>
> 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.
>

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.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151229/f76dc9fb/attachment-0001.html>