<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Aug 20, 2018 at 2:42 AM Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Friday, August 17, 2018 1:06:12 PM PDT Jason Ekstrand wrote:<br>
> ---<br>
>  src/compiler/nir/nir_format_convert.h | 35 +++++++++++++++++++++------<br>
>  1 file changed, 27 insertions(+), 8 deletions(-)<br>
> <br>
> diff --git a/src/compiler/nir/nir_format_convert.h b/src/compiler/nir/nir_format_convert.h<br>
> index b1345f7263b..305273cdfdd 100644<br>
> --- a/src/compiler/nir/nir_format_convert.h<br>
> +++ b/src/compiler/nir/nir_format_convert.h<br>
> @@ -50,6 +50,32 @@ nir_mask_shift_or(struct nir_builder *b, nir_ssa_def *dst, nir_ssa_def *src,<br>
>     return nir_ior(b, nir_mask_shift(b, src, src_mask, src_left_shift), dst);<br>
>  }<br>
>  <br>
> +static inline nir_ssa_def *<br>
> +nir_format_mask_uvec(nir_builder *b, nir_ssa_def *src,<br>
> +                     const unsigned *bits)<br>
> +{<br>
> +   nir_const_value mask;<br>
> +   for (unsigned i = 0; i < src->num_components; i++) {<br>
> +      assert(bits[i] < 32);<br>
> +      mask.u32[i] = (1u << bits[i]) - 1;<br>
> +   }<br>
> +   return nir_iand(b, src, nir_build_imm(b, src->num_components, 32, mask));<br>
> +}<br>
> +<br>
> +static inline nir_ssa_def *<br>
> +nir_format_sign_extend_ivec(nir_builder *b, nir_ssa_def *src,<br>
> +                            const unsigned *bits)<br>
> +{<br>
> +   assert(src->num_components <= 4);<br>
> +   nir_ssa_def *comps[4];<br>
> +   for (unsigned i = 0; i < src->num_components; i++) {<br>
> +      nir_ssa_def *shift = nir_imm_int(b, src->bit_size - bits[i]);<br>
> +      comps[i] = nir_ishr(b, nir_ishl(b, nir_channel(b, src, i), shift), shift);<br>
> +   }<br>
> +   return nir_vec(b, comps, src->num_components);<br>
> +}<br>
> +<br>
> +<br>
>  static inline nir_ssa_def *<br>
>  nir_format_unpack_int(nir_builder *b, nir_ssa_def *packed,<br>
>                        const unsigned *bits, unsigned num_components,<br>
> @@ -117,14 +143,7 @@ static inline nir_ssa_def *<br>
>  nir_format_pack_uint(nir_builder *b, nir_ssa_def *color,<br>
>                       const unsigned *bits, unsigned num_components)<br>
>  {<br>
> -   nir_const_value mask;<br>
> -   for (unsigned i = 0; i < num_components; i++) {<br>
<br>
This used to operate on the num_components parameter to<br>
nir_format_pack_uint, but now it operates on color->num_components<br>
instead.  That's probably OK...do we even need the parameter?<br></blockquote><div><br></div><div>RE masking, yes the new masking helper doesn't take a number of components.  If the number of components in the source SSA def is too small, you're toast; if it's too large, the optimizer will eventually throw away the unneeded instructions.</div><div><br></div><div>For the packing, however, the number of components is needed since it controls how much of the bits array you read and how much you try to cram into the resulting uint32.</div><div><br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Nothing actually uses this function in master today AFAICT...<br>
<br>
Patches 1-3 (with Bas's fixes) and 5-7 are:<br>
Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>><br>
<br>
> -      assert(bits[i] < 32);<br>
> -      mask.u32[i] = (1u << bits[i]) - 1;<br>
> -   }<br>
> -   nir_ssa_def *mask_imm = nir_build_imm(b, num_components, 32, mask);<br>
> -<br>
> -   return nir_format_pack_uint_unmasked(b, nir_iand(b, color, mask_imm),<br>
> +   return nir_format_pack_uint_unmasked(b, nir_format_mask_uvec(b, color, bits),<br>
>                                          bits, num_components);<br>
>  }<br>
>  <br>
> <br>
<br>
</blockquote></div></div>