<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Jul 12, 2018 at 7:20 PM Karol Herbst <<a href="mailto:kherbst@redhat.com">kherbst@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, Jul 13, 2018 at 4:04 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br>
> On Thu, Jul 12, 2018 at 6:48 PM Karol Herbst <<a href="mailto:kherbst@redhat.com" target="_blank">kherbst@redhat.com</a>> wrote:<br>
>><br>
>> There are no fixed sized array arguments in C, those are simply pointers<br>
>> to unsized arrays and as the size is passed in anyway, just rely on that.<br>
>><br>
>> where possible calls are replaced by nir_channel and nir_channels.<br>
>><br>
>> Signed-off-by: Karol Herbst <<a href="mailto:kherbst@redhat.com" target="_blank">kherbst@redhat.com</a>><br>
>> ---<br>
>>  src/amd/vulkan/radv_meta_blit2d.c       |  9 +++------<br>
>>  src/amd/vulkan/radv_meta_buffer.c       |  4 ++--<br>
>>  src/amd/vulkan/radv_query.c             |  6 ++----<br>
>>  src/compiler/nir/nir_builder.h          |  8 ++++----<br>
>>  src/compiler/nir/nir_lower_drawpixels.c |  7 ++-----<br>
>>  src/compiler/nir/nir_lower_tex.c        | 11 +++++------<br>
>>  src/compiler/spirv/spirv_to_nir.c       |  3 +--<br>
>>  src/compiler/spirv/vtn_glsl450.c        | 10 +++++-----<br>
>>  8 files changed, 24 insertions(+), 34 deletions(-)<br>
>><br>
>> diff --git a/src/amd/vulkan/radv_meta_blit2d.c<br>
>> b/src/amd/vulkan/radv_meta_blit2d.c<br>
>> index 85e2d4678e9..cac0a4dd55f 100644<br>
>> --- a/src/amd/vulkan/radv_meta_blit2d.c<br>
>> +++ b/src/amd/vulkan/radv_meta_blit2d.c<br>
>> @@ -608,8 +608,7 @@ build_nir_copy_fragment_shader(struct radv_device<br>
>> *device,<br>
>>         }<br>
>><br>
>>         nir_ssa_def *pos_int = nir_f2i32(&b, nir_load_var(&b,<br>
>> tex_pos_in));<br>
>> -       unsigned swiz[4] = { 0, 1 };<br>
>> -       nir_ssa_def *tex_pos = nir_swizzle(&b, pos_int, swiz, 2, false);<br>
>> +       nir_ssa_def *tex_pos = nir_channels(&b, pos_int, 0x3);<br>
>><br>
>>         nir_ssa_def *color = txf_func(&b, device, tex_pos, is_3d,<br>
>> is_multisampled);<br>
>>         nir_store_var(&b, color_out, color, 0xf);<br>
>> @@ -642,8 +641,7 @@ build_nir_copy_fragment_shader_depth(struct<br>
>> radv_device *device,<br>
>>         }<br>
>><br>
>>         nir_ssa_def *pos_int = nir_f2i32(&b, nir_load_var(&b,<br>
>> tex_pos_in));<br>
>> -       unsigned swiz[4] = { 0, 1 };<br>
>> -       nir_ssa_def *tex_pos = nir_swizzle(&b, pos_int, swiz, 2, false);<br>
>> +       nir_ssa_def *tex_pos = nir_channels(&b, pos_int, 0x3);<br>
>><br>
>>         nir_ssa_def *color = txf_func(&b, device, tex_pos, is_3d,<br>
>> is_multisampled);<br>
>>         nir_store_var(&b, color_out, color, 0x1);<br>
>> @@ -676,8 +674,7 @@ build_nir_copy_fragment_shader_stencil(struct<br>
>> radv_device *device,<br>
>>         }<br>
>><br>
>>         nir_ssa_def *pos_int = nir_f2i32(&b, nir_load_var(&b,<br>
>> tex_pos_in));<br>
>> -       unsigned swiz[4] = { 0, 1 };<br>
>> -       nir_ssa_def *tex_pos = nir_swizzle(&b, pos_int, swiz, 2, false);<br>
>> +       nir_ssa_def *tex_pos = nir_channels(&b, pos_int, 0x3);<br>
>><br>
>>         nir_ssa_def *color = txf_func(&b, device, tex_pos, is_3d,<br>
>> is_multisampled);<br>
>>         nir_store_var(&b, color_out, color, 0x1);<br>
>> diff --git a/src/amd/vulkan/radv_meta_buffer.c<br>
>> b/src/amd/vulkan/radv_meta_buffer.c<br>
>> index c8558216bf1..6c6d1cc41d7 100644<br>
>> --- a/src/amd/vulkan/radv_meta_buffer.c<br>
>> +++ b/src/amd/vulkan/radv_meta_buffer.c<br>
>> @@ -25,7 +25,7 @@ build_buffer_fill_shader(struct radv_device *dev)<br>
>>         nir_ssa_def *global_id = nir_iadd(&b, nir_imul(&b, wg_id,<br>
>> block_size), invoc_id);<br>
>><br>
>>         nir_ssa_def *offset = nir_imul(&b, global_id, nir_imm_int(&b,<br>
>> 16));<br>
>> -       offset = nir_swizzle(&b, offset, (unsigned[]) {0, 0, 0, 0}, 1,<br>
>> false);<br>
>> +       offset = nir_channel(&b, offset, 0);<br>
>><br>
>>         nir_intrinsic_instr *dst_buf =<br>
>> nir_intrinsic_instr_create(b.shader,<br>
>><br>
>> nir_intrinsic_vulkan_resource_index);<br>
>> @@ -77,7 +77,7 @@ build_buffer_copy_shader(struct radv_device *dev)<br>
>>         nir_ssa_def *global_id = nir_iadd(&b, nir_imul(&b, wg_id,<br>
>> block_size), invoc_id);<br>
>><br>
>>         nir_ssa_def *offset = nir_imul(&b, global_id, nir_imm_int(&b,<br>
>> 16));<br>
>> -       offset = nir_swizzle(&b, offset, (unsigned[]) {0, 0, 0, 0}, 1,<br>
>> false);<br>
>> +       offset = nir_channel(&b, offset, 0);<br>
>><br>
>>         nir_intrinsic_instr *dst_buf =<br>
>> nir_intrinsic_instr_create(b.shader,<br>
>><br>
>> nir_intrinsic_vulkan_resource_index);<br>
>> diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c<br>
>> index 16c39aa135d..e3229ab59bb 100644<br>
>> --- a/src/amd/vulkan/radv_query.c<br>
>> +++ b/src/amd/vulkan/radv_query.c<br>
>> @@ -188,10 +188,8 @@ build_occlusion_query_shader(struct radv_device<br>
>> *device) {<br>
>>         load->num_components = 2;<br>
>>         nir_builder_instr_insert(&b, &load->instr);<br>
>><br>
>> -       const unsigned swizzle0[] = {0,0,0,0};<br>
>> -       const unsigned swizzle1[] = {1,1,1,1};<br>
>> -       nir_store_var(&b, start, nir_swizzle(&b, &load->dest.ssa,<br>
>> swizzle0, 1, false), 0x1);<br>
>> -       nir_store_var(&b, end, nir_swizzle(&b, &load->dest.ssa, swizzle1,<br>
>> 1, false), 0x1);<br>
>> +       nir_store_var(&b, start, nir_channel(&b, &load->dest.ssa, 0),<br>
>> 0x1);<br>
>> +       nir_store_var(&b, end, nir_channel(&b, &load->dest.ssa, 1), 0x1);<br>
>><br>
>>         nir_ssa_def *start_done = nir_ilt(&b, nir_load_var(&b, start),<br>
>> nir_imm_int64(&b, 0));<br>
>>         nir_ssa_def *end_done = nir_ilt(&b, nir_load_var(&b, end),<br>
>> nir_imm_int64(&b, 0));<br>
>> diff --git a/src/compiler/nir/nir_builder.h<br>
>> b/src/compiler/nir/nir_builder.h<br>
>> index ae64e72663c..ed61771150a 100644<br>
>> --- a/src/compiler/nir/nir_builder.h<br>
>> +++ b/src/compiler/nir/nir_builder.h<br>
>> @@ -430,12 +430,13 @@ nir_imov_alu(nir_builder *build, nir_alu_src src,<br>
>> unsigned num_components)<br>
>>   * Construct an fmov or imov that reswizzles the source's components.<br>
>>   */<br>
>>  static inline nir_ssa_def *<br>
>> -nir_swizzle(nir_builder *build, nir_ssa_def *src, const unsigned swiz[4],<br>
>> +nir_swizzle(nir_builder *build, nir_ssa_def *src, const unsigned *swiz,<br>
>>              unsigned num_components, bool use_fmov)<br>
>>  {<br>
>> +   assert(num_components <= 4);<br>
>>     nir_alu_src alu_src = { NIR_SRC_INIT };<br>
>>     alu_src.src = nir_src_for_ssa(src);<br>
>> -   for (unsigned i = 0; i < num_components; i++)<br>
>> +   for (unsigned i = 0; i < num_components && i < 4; i++)<br>
><br>
><br>
> What's with the hard-coded 4?<br>
><br>
<br>
bound check on alu_src.swizzle. I put it just in case, I think the<br>
assert should catch all those calls already though. I can also remove<br>
it if you want<br></blockquote><div><br></div><div>That's fine, I just wanted to be sure.  Make sure you replace it with NIR_MAX_COMPONENTS. :-)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>><br>
>>        alu_src.swizzle[i] = swiz[i];<br>
>><br>
>>     return use_fmov ? nir_fmov_alu(build, alu_src, num_components) :<br>
>> @@ -481,8 +482,7 @@ nir_bany(nir_builder *b, nir_ssa_def *src)<br>
>>  static inline nir_ssa_def *<br>
>>  nir_channel(nir_builder *b, nir_ssa_def *def, unsigned c)<br>
>>  {<br>
>> -   unsigned swizzle[4] = {c, c, c, c};<br>
>> -   return nir_swizzle(b, def, swizzle, 1, false);<br>
>> +   return nir_swizzle(b, def, &c, 1, false);<br>
>>  }<br>
>><br>
>>  static inline nir_ssa_def *<br>
>> diff --git a/src/compiler/nir/nir_lower_drawpixels.c<br>
>> b/src/compiler/nir/nir_lower_drawpixels.c<br>
>> index f7ff5c07eaf..462b9c308b2 100644<br>
>> --- a/src/compiler/nir/nir_lower_drawpixels.c<br>
>> +++ b/src/compiler/nir/nir_lower_drawpixels.c<br>
>> @@ -151,9 +151,6 @@ lower_color(lower_drawpixels_state *state,<br>
>> nir_intrinsic_instr *intr)<br>
>>     }<br>
>><br>
>>     if (state->options->pixel_maps) {<br>
>> -      static const unsigned swiz_xy[4] = {0,1};<br>
>> -      static const unsigned swiz_zw[4] = {2,3};<br>
>> -<br>
>>        /* do four pixel map look-ups with two TEX instructions: */<br>
>>        nir_ssa_def *def_xy, *def_zw;<br>
>><br>
>> @@ -166,7 +163,7 @@ lower_color(lower_drawpixels_state *state,<br>
>> nir_intrinsic_instr *intr)<br>
>>        tex->texture_index = state->options->pixelmap_sampler;<br>
>>        tex->dest_type = nir_type_float;<br>
>>        tex->src[0].src_type = nir_tex_src_coord;<br>
>> -      tex->src[0].src = nir_src_for_ssa(nir_swizzle(b, def, swiz_xy, 2,<br>
>> true));<br>
>> +      tex->src[0].src = nir_src_for_ssa(nir_channels(b, def, 0x3));<br>
>><br>
>>        nir_ssa_dest_init(&tex->instr, &tex->dest, 4, 32, NULL);<br>
>>        nir_builder_instr_insert(b, &tex->instr);<br>
>> @@ -180,7 +177,7 @@ lower_color(lower_drawpixels_state *state,<br>
>> nir_intrinsic_instr *intr)<br>
>>        tex->sampler_index = state->options->pixelmap_sampler;<br>
>>        tex->dest_type = nir_type_float;<br>
>>        tex->src[0].src_type = nir_tex_src_coord;<br>
>> -      tex->src[0].src = nir_src_for_ssa(nir_swizzle(b, def, swiz_zw, 2,<br>
>> true));<br>
>> +      tex->src[0].src = nir_src_for_ssa(nir_channels(b, def, 0xc));<br>
>><br>
>>        nir_ssa_dest_init(&tex->instr, &tex->dest, 4, 32, NULL);<br>
>>        nir_builder_instr_insert(b, &tex->instr);<br>
>> diff --git a/src/compiler/nir/nir_lower_tex.c<br>
>> b/src/compiler/nir/nir_lower_tex.c<br>
>> index 1ccd253320c..9f550542fa1 100644<br>
>> --- a/src/compiler/nir/nir_lower_tex.c<br>
>> +++ b/src/compiler/nir/nir_lower_tex.c<br>
>> @@ -479,8 +479,8 @@ lower_gradient_cube_map(nir_builder *b, nir_tex_instr<br>
>> *tex)<br>
>>     nir_ssa_def *cond_z = nir_fge(b, abs_p_z, nir_fmax(b, abs_p_x,<br>
>> abs_p_y));<br>
>>     nir_ssa_def *cond_y = nir_fge(b, abs_p_y, nir_fmax(b, abs_p_x,<br>
>> abs_p_z));<br>
>><br>
>> -   unsigned yzx[4] = { 1, 2, 0, 0 };<br>
>> -   unsigned xzy[4] = { 0, 2, 1, 0 };<br>
>> +   unsigned yzx[3] = { 1, 2, 0 };<br>
>> +   unsigned xzy[3] = { 0, 2, 1 };<br>
>><br>
>>     Q = nir_bcsel(b, cond_z,<br>
>>                   p,<br>
>> @@ -508,16 +508,15 @@ lower_gradient_cube_map(nir_builder *b,<br>
>> nir_tex_instr *tex)<br>
>>      */<br>
>>     nir_ssa_def *rcp_Q_z = nir_frcp(b, nir_channel(b, Q, 2));<br>
>><br>
>> -   unsigned xy[4] = { 0, 1, 0, 0 };<br>
>> -   nir_ssa_def *Q_xy = nir_swizzle(b, Q, xy, 2, false);<br>
>> +   nir_ssa_def *Q_xy = nir_channels(b, Q, 0x3);<br>
>>     nir_ssa_def *tmp = nir_fmul(b, Q_xy, rcp_Q_z);<br>
>><br>
>> -   nir_ssa_def *dQdx_xy = nir_swizzle(b, dQdx, xy, 2, false);<br>
>> +   nir_ssa_def *dQdx_xy = nir_channels(b, dQdx, 0x3);<br>
>>     nir_ssa_def *dQdx_z = nir_channel(b, dQdx, 2);<br>
>>     nir_ssa_def *dx =<br>
>>        nir_fmul(b, rcp_Q_z, nir_fsub(b, dQdx_xy, nir_fmul(b, tmp,<br>
>> dQdx_z)));<br>
>><br>
>> -   nir_ssa_def *dQdy_xy = nir_swizzle(b, dQdy, xy, 2, false);<br>
>> +   nir_ssa_def *dQdy_xy = nir_channels(b, dQdy, 0x3);<br>
>>     nir_ssa_def *dQdy_z = nir_channel(b, dQdy, 2);<br>
>>     nir_ssa_def *dy =<br>
>>        nir_fmul(b, rcp_Q_z, nir_fsub(b, dQdy_xy, nir_fmul(b, tmp,<br>
>> dQdy_z)));<br>
>> diff --git a/src/compiler/spirv/spirv_to_nir.c<br>
>> b/src/compiler/spirv/spirv_to_nir.c<br>
>> index 235003e872a..48154303ff2 100644<br>
>> --- a/src/compiler/spirv/spirv_to_nir.c<br>
>> +++ b/src/compiler/spirv/spirv_to_nir.c<br>
>> @@ -2858,8 +2858,7 @@ vtn_ssa_transpose(struct vtn_builder *b, struct<br>
>> vtn_ssa_value *src)<br>
>>  nir_ssa_def *<br>
>>  vtn_vector_extract(struct vtn_builder *b, nir_ssa_def *src, unsigned<br>
>> index)<br>
>>  {<br>
>> -   unsigned swiz[4] = { index };<br>
>> -   return nir_swizzle(&b->nb, src, swiz, 1, false);<br>
>> +   return nir_channel(&b->nb, src, index);<br>
>>  }<br>
>><br>
>>  nir_ssa_def *<br>
>> diff --git a/src/compiler/spirv/vtn_glsl450.c<br>
>> b/src/compiler/spirv/vtn_glsl450.c<br>
>> index 8393450f2f4..9ef066e8e6d 100644<br>
>> --- a/src/compiler/spirv/vtn_glsl450.c<br>
>> +++ b/src/compiler/spirv/vtn_glsl450.c<br>
>> @@ -36,7 +36,7 @@<br>
>>  static nir_ssa_def *<br>
>>  build_mat2_det(nir_builder *b, nir_ssa_def *col[2])<br>
>>  {<br>
>> -   unsigned swiz[4] = {1, 0, 0, 0};<br>
>> +   unsigned swiz[2] = {1, 0 };<br>
>>     nir_ssa_def *p = nir_fmul(b, col[0], nir_swizzle(b, col[1], swiz, 2,<br>
>> true));<br>
><br>
><br>
> nir_channels?<br>
><br>
<br>
it selects .yx, no? I am sure nir_channels can't do that.<br></blockquote><div><br></div><div>Drp... I failed at reading.  Patch is<br></div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>><br>
>>     return nir_fsub(b, nir_channel(b, p, 0), nir_channel(b, p, 1));<br>
>>  }<br>
>> @@ -44,8 +44,8 @@ build_mat2_det(nir_builder *b, nir_ssa_def *col[2])<br>
>>  static nir_ssa_def *<br>
>>  build_mat3_det(nir_builder *b, nir_ssa_def *col[3])<br>
>>  {<br>
>> -   unsigned yzx[4] = {1, 2, 0, 0};<br>
>> -   unsigned zxy[4] = {2, 0, 1, 0};<br>
>> +   unsigned yzx[3] = {1, 2, 0 };<br>
>> +   unsigned zxy[3] = {2, 0, 1 };<br>
>><br>
>>     nir_ssa_def *prod0 =<br>
>>        nir_fmul(b, col[0],<br>
>> @@ -602,8 +602,8 @@ handle_glsl450_alu(struct vtn_builder *b, enum<br>
>> GLSLstd450 entrypoint,<br>
>>        return;<br>
>><br>
>>     case GLSLstd450Cross: {<br>
>> -      unsigned yzx[4] = { 1, 2, 0, 0 };<br>
>> -      unsigned zxy[4] = { 2, 0, 1, 0 };<br>
>> +      unsigned yzx[3] = { 1, 2, 0 };<br>
>> +      unsigned zxy[3] = { 2, 0, 1 };<br>
>>        val->ssa->def =<br>
>>           nir_fsub(nb, nir_fmul(nb, nir_swizzle(nb, src[0], yzx, 3, true),<br>
>>                                     nir_swizzle(nb, src[1], zxy, 3,<br>
>> true)),<br>
>> --<br>
>> 2.17.1<br>
>><br>
>> _______________________________________________<br>
>> mesa-dev mailing list<br>
>> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
>> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div>