Patches 1, 2 & 4 - 10: Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
For 8, I replied with a non-essential question, and the same question is relevant in patch 10. For patch 3 I had that question (mentioned on irc) about ir having ast/parser knowledge. I guess I'd like to think about this one a little more. -Jordan On Fri, Sep 27, 2013 at 12:05 PM, Paul Berry <stereotype...@gmail.com> wrote: > Although it's not explicitly stated in the GLSL 1.50 spec, it is > clearly implied that unsized arrays are allowed in interface blocks > (the built-in interface gl_PerVertex contains an unsized array, > gl_ClipDistance, and examples added in GLSL 4.30 make use of unsized > arrays in interface blocks). > > From the client's point of view, unsized inside interface blocks work > the same way as unsized arrays outside of interface blocks: they > acquire a size at link time based on the maximum array element > accessed (across all compilation units for a given shader stage), and > then this acquired size is used to match interfaces from one shader > stage to the next. > > Internally, however, things are a bit different, because Mesa's > implementation of interface blocks makes heavy re-use of its > implementation of structs. Ideally I would have liked to keep track > of the maximum array element accessed inside the data structure that > stores the interface type, and I would have liked to update this data > structure in place when the array sizes are acquired. But this is > unsafe because the implementation uses a flyweight pattern for types; > at the time a struct or interface type is declared, it is looked up in > a global hash table, and if a matching struct or interface type has > previously been declared, the pointer is shared (even if the matching > struct was used in a different shader, different thread, or different > context). As a result, any interface type in existence may be shared > with an unknown set of shaders (some of which are possibly being > compiled concurrently), so it is unsafe to modify the data structure. > Therefore, instead, we track the maximum array element accessed inside > the ir_variable class, and when the arrays acquire sizes we create a > new interface type and walk through the IR updating the type of all > the variables that refer to it. > > Patches 1-4 are preparatory work. Patch 1 adds parser support for > unsized arrays in interface blocks. Patches 2 and 3 do some minor > code reorganization so that in patch 7, we will be able to easily > track the maximum element used in any array that appears in an > interface block. Patch 4 makes ir_variable::interface_type private > and creates getter and setter methods for it; in patches 5 and 8, this > will allow us to ensure than an invariant is preserved when the > interface type changes. > > Patch 5 adds a new field, ir_variable::max_ifc_array_access, which > keeps track of the maximum element used in any array that appears in > an interface block. Patch 6 adds code to ir_validate to make sure > that the values in max_ifc_array_access are never greater than the > sizes of sized arrays. Patch 7 adds code to update > max_ifc_array_access properly when an array is accessed. > > Patch 8 modifies the linker's array_sizing_visitor so that unsized > arrays in named interface blocks acquire sizes at link time. Patch 9 > modifies the linker's call_link_visitor so that if an interface block > containing unsized arrays is used in multiple compilation units for > the same shader stage, we use the maximum array access across all > compilation units to assign the size. Patch 10 modifies the linker's > array_sizing_visitor so that unsized arrays in unnamed interface > blocks acquire sizes at link time. > > > [PATCH 01/10] glsl: Add parser support for unsized arrays in interface blocks. > [PATCH 02/10] glsl: Link uniform-initializer-test with > standalone_scaffolding.cpp. > [PATCH 03/10] glsl: Move update of max_array_access into a virtual function > on ir_rvalue. > [PATCH 04/10] glsl: Make accessor functions for ir_variable::interface_type. > [PATCH 05/10] glsl: Add an ir_variable::max_ifc_array_access field. > [PATCH 06/10] glsl: Sanity check max_ifc_array_access in > ir_validate::visit(ir_variable *). > [PATCH 07/10] glsl: Update ir_variable::max_ifc_array_access properly. > [PATCH 08/10] glsl/linker: Modify array_sizing_visitor to handle named > interface blocks. > [PATCH 09/10] glsl: Update call_link_visitor to update max_ifc_array_access. > [PATCH 10/10] glsl: Modify array_sizing_visitor to handle unnamed interface > blocks. > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
