[Mesa-dev] [PATCH 3/6] glsl: Generalize compute_packing_order for varying structs.

[Paul Berry]

On 23 January 2013 08:36, Ian Romanick <idr at freedesktop.org> wrote:

> On 01/23/2013 11:16 AM, Paul Berry wrote:
>
>> On 23 January 2013 07:38, Ian Romanick <idr at freedesktop.org
>> <mailto:idr at freedesktop.org>> wrote:
>>
>>     On 01/21/2013 05:16 PM, Paul Berry wrote:
>>
>>         This patch paves the way for allowing varying structs by
>>         generalizing
>>         varying_matches::compute___**packing_order to handle any type of
>>
>>         varying.
>>         Previously, we packed in the order (vec4, vec2, float, vec3), with
>>         matrices being packed according to the size of their columns.
>>           Now, we
>>         pack everything according to its number of components mod 4, in
>> the
>>         order (0, 2, 1, 3).
>>
>>         There is no behavioural change for vectors.  Matrices are now
>> packed
>>         slightly more sensibly (e.g. a mat2 is now packed with vec4's,
>> which
>>         guarantees that it will occupy a single varying slot, whereas it
>>         previously was packed with vec2's, which meant that it might get
>>         split
>>         across two adjacent slots).
>>         ---
>>            src/glsl/link_varyings.cpp | 5 ++---
>>            1 file changed, 2 insertions(+), 3 deletions(-)
>>
>>         diff --git a/src/glsl/link_varyings.cpp
>> b/src/glsl/link_varyings.cpp
>>         index 5a3240b..d8f501c 100644
>>         --- a/src/glsl/link_varyings.cpp
>>         +++ b/src/glsl/link_varyings.cpp
>>         @@ -804,16 +804,15 @@
>>         varying_matches::compute___**packing_order(ir_variable *var)
>>
>>            {
>>               const glsl_type *element_type = var->type;
>>
>>         -   /* FINISHME: Support for "varying" records in GLSL 1.50. */
>>               while (element_type->base_type == GLSL_TYPE_ARRAY) {
>>                  element_type = element_type->fields.array;
>>               }
>>
>>         -   switch (element_type->vector___**elements) {
>>         +   switch (element_type->component___**slots() % 4) {
>>
>>
>>
>>     Will this produce the same result for all matrix types as well?  I
>>     think a mat2x3 would be 3 in the old scheme, but 2 in the new
>>     scheme.  Right?
>>
>>
>> Yes, a number of matrix types will get new packing orders.  Here's the
>> complete list:
>>
>> - mat2x2 gets assigned PACKING_ORDER_VEC4 instead of
>> PACKING_ORDER_VEC2.  As I argue above, this is slightly better, because
>> it guarantees that the matrix occupies a single varying slot.
>>
>> - mat2x3 gets assigned PACKING_ORDER_VEC2 instead of
>> PACKING_ORDER_VEC3.  This is kind of a wash.  Previously, mat2x3 had a
>> 25% chance of having neither of its columns double parked, a 50% chance
>> of having exactly one of its columns double parked, and a 25% chance of
>> having both of its columns double parked.  Now it always has exactly one
>> of its columns double parked.
>>
>> - mat3x3 gets assigned PACKING_ORDER_SCALAR instead of
>> PACKING_ORDER_VEC3.  This doesn't affect much, since in both cases there
>> is no guarantee of how the matrix will be aligned.
>>
>> - mat4x2 gets assigned PACKING_ORDER_VEC4 instead of
>> PACKING_ORDER_VEC2.  I think this is slightly better for the same reason
>> as in mat2x2.
>>
>> - mat4x3 gets assigned PACKING_ORDER_VEC4 instead of
>> PACKING_ORDER_VEC3.  I think this is slightly better for the same reason
>> as in mat2x2.
>>
>
> These changes potentially affect shader performance, do they also affect
> the number of slots used?  In other words, will a different set of shaders
> fit vs. not fit in 16 vec4 slots?


No, they just affect the order of packing.  Packing is always done without
gaps, so the same set of shaders will fit vs. not fit as before.


>
>
>  Do you think I should include these details in the commit message?
>>
>
> Since you've already written it, the cut-and-paste couldn't hurt.


Ok, will do.


>
>
>                case 1: return PACKING_ORDER_SCALAR;
>>               case 2: return PACKING_ORDER_VEC2;
>>               case 3: return PACKING_ORDER_VEC3;
>>         -   case 4: return PACKING_ORDER_VEC4;
>>         +   case 0: return PACKING_ORDER_VEC4;
>>               default:
>>                  assert(!"Unexpected value of vector_elements");
>>                  return PACKING_ORDER_VEC4;
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130123/376d3ac4/attachment.html>