[Piglit] [PATCH] builtin_packing: Add swizzles to the sources of packing functions

Ian Romanick idr at freedesktop.org
Thu Oct 11 21:24:39 UTC 2018


On 10/11/2018 02:09 PM, Jason Ekstrand wrote:
> commit dd553bc67f8ab1513fd196b6ffb7c4a76723adfd
> Author: Jason Ekstrand <jason.ekstrand at intel.com
> <mailto:jason.ekstrand at intel.com>>
> Date:   Wed Oct 3 12:14:20 2018 -0500
> 
>     nir/alu_to_scalar: Use ssa_for_alu_src in hand-rolled expansions
>    
>     The ssa_for_alu_src helper will correctly handle swizzles and other
>     source modifiers for you.  The expansions for unpack_half_2x16,
>     pack_uvec2_to_uint, and pack_uvec4_to_uint were all broken with regards
>     to swizzles.  The brokenness of unpack_half_2x16 was causing rendering
>     errors in Rise of the Tomb Raider on Intel ever since c11833ab24dcba26
>     which added an extra copy propagation to the optimization pipeline and
>     caused us to start seeing swizzles where we hadn't seen any before.
>    
>     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107926
>     Fixes: 9ce901058f3d "nir: Add lowering of nir_op_unpack_half_2x16."
>     Fixes: 9b8786eba955 "nir: Add lowering support for packing opcodes."
>     Tested-by: Alex Smith <asmith at feralinteractive.com
> <mailto:asmith at feralinteractive.com>>
>     Tested-by: Józef Kucia <joseph.kucia at gmail.com
> <mailto:joseph.kucia at gmail.com>>
>     Reviewed-by: Matt Turner <mattst88 at gmail.com
> <mailto:mattst88 at gmail.com>>

I'll take that as, "Yes." ;)  If you add a reference to the bug in the
commit message (so that nobody comes along later to "optimize" the tests),

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> On Wed, Oct 10, 2018 at 1:45 PM Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
> 
>     These changes look good, but I'm wondering why. :)  Did you find some
>     bug, or ... ?
> 
>     On 10/04/2018 10:42 AM, Jason Ekstrand wrote:
>     > ---
>     >  .../fs_pack.shader_test.mako                       | 14
>     +++++++++++---
>     >  .../fs_unpack.shader_test.mako                     |  6 +++---
>     >  .../vs_pack.shader_test.mako                       | 14
>     +++++++++++---
>     >  .../vs_unpack.shader_test.mako                     |  6 +++---
>     >  4 files changed, 28 insertions(+), 12 deletions(-)
>     >
>     > diff --git
>     a/generated_tests/templates/gen_builtin_packing_tests/fs_pack.shader_test.mako
>     b/generated_tests/templates/gen_builtin_packing_tests/fs_pack.shader_test.mako
>     > index 4c053bf89..86c63061e 100644
>     > ---
>     a/generated_tests/templates/gen_builtin_packing_tests/fs_pack.shader_test.mako
>     > +++
>     b/generated_tests/templates/gen_builtin_packing_tests/fs_pack.shader_test.mako
>     > @@ -21,7 +21,7 @@ precision highp float;
>     >  const vec4 red = vec4(1, 0, 0, 1);
>     >  const vec4 green = vec4(0, 1, 0, 1);
>>     > -uniform ${func.vector_type} func_input;
>     > +uniform vec4 func_input;
>>     >  % for i in range(func.num_valid_outputs):
>     >  uniform ${func.result_precision} uint expect${i};
>     > @@ -31,7 +31,11 @@ out vec4 frag_color;
>>     >  void main()
>     >  {
>     > -    ${func.result_precision} uint actual = ${func.name
>     <http://func.name>}(func_input);
>     > +% if func.vector_type == 'vec2':
>     > +    ${func.result_precision} uint actual = ${func.name
>     <http://func.name>}(func_input.wx);
>     > +% else:
>     > +    ${func.result_precision} uint actual = ${func.name
>     <http://func.name>}(func_input.wxzy);
>     > +% endif
>>     >      if (false
>     >          % for i in range(func.num_valid_outputs):
>     > @@ -53,7 +57,11 @@ vertex/float/2
>>     >  [test]
>     >  % for io in func.inout_seq:
>     > -uniform ${func.vector_type} func_input ${" ".join(io.input)}
>     > +% if func.vector_type == 'vec2':
>     > +uniform vec4 func_input ${io.input[1]} 0.0 0.0 ${io.input[0]}
>     > +% else:
>     > +uniform vec4 func_input ${io.input[1]} ${io.input[3]}
>     ${io.input[2]} ${io.input[0]}
>     > +% endif
>     >  % for i in range(func.num_valid_outputs):
>     >  uniform uint expect${i} ${io.valid_outputs[i]}
>     >  % endfor
>     > diff --git
>     a/generated_tests/templates/gen_builtin_packing_tests/fs_unpack.shader_test.mako
>     b/generated_tests/templates/gen_builtin_packing_tests/fs_unpack.shader_test.mako
>     > index ee610bbfb..8e3f12a58 100644
>     > ---
>     a/generated_tests/templates/gen_builtin_packing_tests/fs_unpack.shader_test.mako
>     > +++
>     b/generated_tests/templates/gen_builtin_packing_tests/fs_unpack.shader_test.mako
>     > @@ -21,7 +21,7 @@ precision highp float;
>     >  const vec4 red = vec4(1, 0, 0, 1);
>     >  const vec4 green = vec4(0, 1, 0, 1);
>>     > -uniform highp uint func_input;
>     > +uniform highp uvec4 func_input;
>>     >  uniform bool exact;
>>     > @@ -33,7 +33,7 @@ out vec4 frag_color;
>>     >  void main()
>     >  {
>     > -    ${func.result_precision} ${func.vector_type} actual =
>     ${func.name <http://func.name>}(func_input);
>     > +    ${func.result_precision} ${func.vector_type} actual =
>     ${func.name <http://func.name>}(func_input.y);
>>     >      if (false
>     >          % for i in range(func.num_valid_outputs):
>     > @@ -56,7 +56,7 @@ vertex/float/2
>>     >  [test]
>     >  % for io in func.inout_seq:
>     > -uniform uint func_input ${io.input}
>     > +uniform uvec4 func_input 0 ${io.input} 0 0
>     >  % if func.exact:
>     >  uniform int exact 1
>     >  % else:
>     > diff --git
>     a/generated_tests/templates/gen_builtin_packing_tests/vs_pack.shader_test.mako
>     b/generated_tests/templates/gen_builtin_packing_tests/vs_pack.shader_test.mako
>     > index 5f37d7eaf..81c725846 100644
>     > ---
>     a/generated_tests/templates/gen_builtin_packing_tests/vs_pack.shader_test.mako
>     > +++
>     b/generated_tests/templates/gen_builtin_packing_tests/vs_pack.shader_test.mako
>     > @@ -11,7 +11,7 @@ ${func.requirements}
>     >  const vec4 red = vec4(1, 0, 0, 1);
>     >  const vec4 green = vec4(0, 1, 0, 1);
>>     > -uniform ${func.vector_type} func_input;
>     > +uniform vec4 func_input;
>>     >  % for j in range(func.num_valid_outputs):
>     >  uniform ${func.result_precision} uint expect${j};
>     > @@ -23,7 +23,11 @@ out vec4 vert_color;
>     >  void main()
>     >  {
>     >      gl_Position = vertex;
>     > -    ${func.result_precision} uint actual = ${func.name
>     <http://func.name>}(func_input);
>     > +% if func.vector_type == 'vec2':
>     > +    ${func.result_precision} uint actual = ${func.name
>     <http://func.name>}(func_input.zy);
>     > +% else:
>     > +    ${func.result_precision} uint actual = ${func.name
>     <http://func.name>}(func_input.wyzx);
>     > +% endif
>>     >      if (false
>     >          % for j in range(func.num_valid_outputs):
>     > @@ -58,7 +62,11 @@ vertex/float/2
>>     >  [test]
>     >  % for io in func.inout_seq:
>     > -uniform ${func.vector_type} func_input ${" ".join(io.input)}
>     > +% if func.vector_type == 'vec2':
>     > +uniform vec4 func_input 0.0 ${io.input[1]} ${io.input[0]} 0.0
>     > +% else:
>     > +uniform vec4 func_input ${io.input[3]} ${io.input[1]}
>     ${io.input[2]} ${io.input[0]}
>     > +% endif
>     >  % for j in range(func.num_valid_outputs):
>     >  uniform uint expect${j} ${io.valid_outputs[j]}
>     >  % endfor
>     > diff --git
>     a/generated_tests/templates/gen_builtin_packing_tests/vs_unpack.shader_test.mako
>     b/generated_tests/templates/gen_builtin_packing_tests/vs_unpack.shader_test.mako
>     > index 5f2a39125..c14fd35a0 100644
>     > ---
>     a/generated_tests/templates/gen_builtin_packing_tests/vs_unpack.shader_test.mako
>     > +++
>     b/generated_tests/templates/gen_builtin_packing_tests/vs_unpack.shader_test.mako
>     > @@ -11,7 +11,7 @@ ${func.requirements}
>     >  const vec4 red = vec4(1, 0, 0, 1);
>     >  const vec4 green = vec4(0, 1, 0, 1);
>>     > -uniform highp uint func_input;
>     > +uniform highp uvec4 func_input;
>>     >  uniform bool exact;
>>     > @@ -26,7 +26,7 @@ void main()
>     >  {
>     >      gl_Position = vertex;
>>     > -    ${func.result_precision} ${func.vector_type} actual =
>     ${func.name <http://func.name>}(func_input);
>     > +    ${func.result_precision} ${func.vector_type} actual =
>     ${func.name <http://func.name>}(func_input.z);
>>     >      if (false
>     >          % for i in range(func.num_valid_outputs):
>     > @@ -61,7 +61,7 @@ vertex/float/2
>>     >  [test]
>     >  % for io in func.inout_seq:
>     > -uniform uint func_input ${io.input}
>     > +uniform uvec4 func_input 0 0 ${io.input} 0
>     >  % if func.exact:
>     >  uniform int exact 1
>     >  % else:
>     >
> 



More information about the Piglit mailing list