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