Re: [Mesa-dev] [PATCH 5/6] nir: prepare for bumping up max components to 16
I made some nitpick comments below. Patches 1-5 are
Reviewed-by: Jason Ekstrand
On Sat, Jul 14, 2018 at 7:56 PM Jason Ekstrand wrote:
> On Thu, Jul 12, 2018 at 4:30 AM Karol Herbst wrote:
>
>> OpenCL knows vector of size 8 and 16.
>>
>> Signed-off-by: Karol Herbst
>> ---
>> src/compiler/nir/nir.c| 14
>> src/compiler/nir/nir.h| 34 ++-
>> src/compiler/nir/nir_builder.h| 18 ++
>> src/compiler/nir/nir_lower_alu_to_scalar.c| 6 ++--
>> src/compiler/nir/nir_lower_io_to_scalar.c | 4 +--
>> .../nir/nir_lower_load_const_to_scalar.c | 2 +-
>> src/compiler/nir/nir_opt_constant_folding.c | 2 +-
>> src/compiler/nir/nir_opt_copy_prop_vars.c | 4 +--
>> src/compiler/nir/nir_print.c | 8 ++---
>> src/compiler/nir/nir_search.c | 8 ++---
>> src/compiler/nir/nir_validate.c | 6 ++--
>> src/compiler/spirv/spirv_to_nir.c | 4 +--
>> src/compiler/spirv/vtn_alu.c | 2 +-
>> 13 files changed, 59 insertions(+), 53 deletions(-)
>>
>> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
>> index ca89a46f7d4..bc7f05b3e86 100644
>> --- a/src/compiler/nir/nir.c
>> +++ b/src/compiler/nir/nir.c
>> @@ -251,7 +251,7 @@ nir_alu_src_copy(nir_alu_src *dest, const nir_alu_src
>> *src,
>> nir_src_copy(>src, >src, >instr);
>> dest->abs = src->abs;
>> dest->negate = src->negate;
>> - for (unsigned i = 0; i < 4; i++)
>> + for (unsigned i = 0; i < NIR_MAX_VEC_COMPONENTS; i++)
>>dest->swizzle[i] = src->swizzle[i];
>> }
>>
>> @@ -421,10 +421,8 @@ alu_src_init(nir_alu_src *src)
>> {
>> src_init(>src);
>> src->abs = src->negate = false;
>> - src->swizzle[0] = 0;
>> - src->swizzle[1] = 1;
>> - src->swizzle[2] = 2;
>> - src->swizzle[3] = 3;
>> + for (int i = 0; i < NIR_MAX_VEC_COMPONENTS; ++i)
>> + src->swizzle[i] = i;
>> }
>>
>> nir_alu_instr *
>> @@ -1426,10 +1424,10 @@ nir_ssa_def_rewrite_uses_after(nir_ssa_def *def,
>> nir_src new_src,
>>nir_if_rewrite_condition(use_src->parent_if, new_src);
>> }
>>
>> -uint8_t
>> +nir_component_mask_t
>> nir_ssa_def_components_read(const nir_ssa_def *def)
>> {
>> - uint8_t read_mask = 0;
>> + nir_component_mask_t read_mask = 0;
>> nir_foreach_use(use, def) {
>>if (use->parent_instr->type == nir_instr_type_alu) {
>> nir_alu_instr *alu = nir_instr_as_alu(use->parent_instr);
>> @@ -1437,7 +1435,7 @@ nir_ssa_def_components_read(const nir_ssa_def *def)
>> int src_idx = alu_src - >src[0];
>> assert(src_idx >= 0 && src_idx <
>> nir_op_infos[alu->op].num_inputs);
>>
>> - for (unsigned c = 0; c < 4; c++) {
>> + for (unsigned c = 0; c < NIR_MAX_VEC_COMPONENTS; c++) {
>> if (!nir_alu_instr_channel_used(alu, src_idx, c))
>> continue;
>>
>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>> index 92ab3a699cc..d3e63be091f 100644
>> --- a/src/compiler/nir/nir.h
>> +++ b/src/compiler/nir/nir.h
>> @@ -57,6 +57,8 @@ extern "C" {
>>
>> #define NIR_FALSE 0u
>> #define NIR_TRUE (~0u)
>> +#define NIR_MAX_VEC_COMPONENTS 4
>> +typedef uint8_t nir_component_mask_t;
>>
>> /** Defines a cast function
>> *
>> @@ -115,16 +117,16 @@ typedef enum {
>> } nir_rounding_mode;
>>
>> typedef union {
>> - float f32[4];
>> - double f64[4];
>> - int8_t i8[4];
>> - uint8_t u8[4];
>> - int16_t i16[4];
>> - uint16_t u16[4];
>> - int32_t i32[4];
>> - uint32_t u32[4];
>> - int64_t i64[4];
>> - uint64_t u64[4];
>> + float f32[NIR_MAX_VEC_COMPONENTS];
>> + double f64[NIR_MAX_VEC_COMPONENTS];
>> + int8_t i8[NIR_MAX_VEC_COMPONENTS];
>> + uint8_t u8[NIR_MAX_VEC_COMPONENTS];
>> + int16_t i16[NIR_MAX_VEC_COMPONENTS];
>> + uint16_t u16[NIR_MAX_VEC_COMPONENTS];
>> + int32_t i32[NIR_MAX_VEC_COMPONENTS];
>> + uint32_t u32[NIR_MAX_VEC_COMPONENTS];
>> + int64_t i64[NIR_MAX_VEC_COMPONENTS];
>> + uint64_t u64[NIR_MAX_VEC_COMPONENTS];
>> } nir_const_value;
>>
>> typedef struct nir_constant {
>> @@ -135,7 +137,7 @@ typedef struct nir_constant {
>> * by the type associated with the \c nir_variable. Constants may be
>> * scalars, vectors, or matrices.
>> */
>> - nir_const_value values[4];
>> + nir_const_value values[NIR_MAX_VEC_COMPONENTS];
>>
>> /* we could get this from the var->type but makes clone *much* easier
>> to
>> * not have to care about the type.
>> @@ -697,7 +699,7 @@ typedef struct {
>> * a statement like "foo.xzw = bar.zyx" would have a writemask of
>> 1101b and
>> * a swizzle of {2, x, 1, 0} where x means "don't care."
>> */
>> - uint8_t swizzle[4];
>> + uint8_t swizzle[NIR_MAX_VEC_COMPONENTS];
>> } nir_alu_src;
>>
>> typedef struct {
>> @@ -712,7 +714,7 @@ typedef struct {
>>
>> bool saturate;
>>
>> - unsigned write_mask : 4; /* ignored if dest.is_ssa is true */
>> +
Re: [Mesa-dev] [PATCH 5/6] nir: prepare for bumping up max components to 16
On Thu, Jul 12, 2018 at 4:30 AM Karol Herbst wrote:
> OpenCL knows vector of size 8 and 16.
>
> Signed-off-by: Karol Herbst
> ---
> src/compiler/nir/nir.c| 14
> src/compiler/nir/nir.h| 34 ++-
> src/compiler/nir/nir_builder.h| 18 ++
> src/compiler/nir/nir_lower_alu_to_scalar.c| 6 ++--
> src/compiler/nir/nir_lower_io_to_scalar.c | 4 +--
> .../nir/nir_lower_load_const_to_scalar.c | 2 +-
> src/compiler/nir/nir_opt_constant_folding.c | 2 +-
> src/compiler/nir/nir_opt_copy_prop_vars.c | 4 +--
> src/compiler/nir/nir_print.c | 8 ++---
> src/compiler/nir/nir_search.c | 8 ++---
> src/compiler/nir/nir_validate.c | 6 ++--
> src/compiler/spirv/spirv_to_nir.c | 4 +--
> src/compiler/spirv/vtn_alu.c | 2 +-
> 13 files changed, 59 insertions(+), 53 deletions(-)
>
> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
> index ca89a46f7d4..bc7f05b3e86 100644
> --- a/src/compiler/nir/nir.c
> +++ b/src/compiler/nir/nir.c
> @@ -251,7 +251,7 @@ nir_alu_src_copy(nir_alu_src *dest, const nir_alu_src
> *src,
> nir_src_copy(>src, >src, >instr);
> dest->abs = src->abs;
> dest->negate = src->negate;
> - for (unsigned i = 0; i < 4; i++)
> + for (unsigned i = 0; i < NIR_MAX_VEC_COMPONENTS; i++)
>dest->swizzle[i] = src->swizzle[i];
> }
>
> @@ -421,10 +421,8 @@ alu_src_init(nir_alu_src *src)
> {
> src_init(>src);
> src->abs = src->negate = false;
> - src->swizzle[0] = 0;
> - src->swizzle[1] = 1;
> - src->swizzle[2] = 2;
> - src->swizzle[3] = 3;
> + for (int i = 0; i < NIR_MAX_VEC_COMPONENTS; ++i)
> + src->swizzle[i] = i;
> }
>
> nir_alu_instr *
> @@ -1426,10 +1424,10 @@ nir_ssa_def_rewrite_uses_after(nir_ssa_def *def,
> nir_src new_src,
>nir_if_rewrite_condition(use_src->parent_if, new_src);
> }
>
> -uint8_t
> +nir_component_mask_t
> nir_ssa_def_components_read(const nir_ssa_def *def)
> {
> - uint8_t read_mask = 0;
> + nir_component_mask_t read_mask = 0;
> nir_foreach_use(use, def) {
>if (use->parent_instr->type == nir_instr_type_alu) {
> nir_alu_instr *alu = nir_instr_as_alu(use->parent_instr);
> @@ -1437,7 +1435,7 @@ nir_ssa_def_components_read(const nir_ssa_def *def)
> int src_idx = alu_src - >src[0];
> assert(src_idx >= 0 && src_idx <
> nir_op_infos[alu->op].num_inputs);
>
> - for (unsigned c = 0; c < 4; c++) {
> + for (unsigned c = 0; c < NIR_MAX_VEC_COMPONENTS; c++) {
> if (!nir_alu_instr_channel_used(alu, src_idx, c))
> continue;
>
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 92ab3a699cc..d3e63be091f 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -57,6 +57,8 @@ extern "C" {
>
> #define NIR_FALSE 0u
> #define NIR_TRUE (~0u)
> +#define NIR_MAX_VEC_COMPONENTS 4
> +typedef uint8_t nir_component_mask_t;
>
> /** Defines a cast function
> *
> @@ -115,16 +117,16 @@ typedef enum {
> } nir_rounding_mode;
>
> typedef union {
> - float f32[4];
> - double f64[4];
> - int8_t i8[4];
> - uint8_t u8[4];
> - int16_t i16[4];
> - uint16_t u16[4];
> - int32_t i32[4];
> - uint32_t u32[4];
> - int64_t i64[4];
> - uint64_t u64[4];
> + float f32[NIR_MAX_VEC_COMPONENTS];
> + double f64[NIR_MAX_VEC_COMPONENTS];
> + int8_t i8[NIR_MAX_VEC_COMPONENTS];
> + uint8_t u8[NIR_MAX_VEC_COMPONENTS];
> + int16_t i16[NIR_MAX_VEC_COMPONENTS];
> + uint16_t u16[NIR_MAX_VEC_COMPONENTS];
> + int32_t i32[NIR_MAX_VEC_COMPONENTS];
> + uint32_t u32[NIR_MAX_VEC_COMPONENTS];
> + int64_t i64[NIR_MAX_VEC_COMPONENTS];
> + uint64_t u64[NIR_MAX_VEC_COMPONENTS];
> } nir_const_value;
>
> typedef struct nir_constant {
> @@ -135,7 +137,7 @@ typedef struct nir_constant {
> * by the type associated with the \c nir_variable. Constants may be
> * scalars, vectors, or matrices.
> */
> - nir_const_value values[4];
> + nir_const_value values[NIR_MAX_VEC_COMPONENTS];
>
> /* we could get this from the var->type but makes clone *much* easier
> to
> * not have to care about the type.
> @@ -697,7 +699,7 @@ typedef struct {
> * a statement like "foo.xzw = bar.zyx" would have a writemask of
> 1101b and
> * a swizzle of {2, x, 1, 0} where x means "don't care."
> */
> - uint8_t swizzle[4];
> + uint8_t swizzle[NIR_MAX_VEC_COMPONENTS];
> } nir_alu_src;
>
> typedef struct {
> @@ -712,7 +714,7 @@ typedef struct {
>
> bool saturate;
>
> - unsigned write_mask : 4; /* ignored if dest.is_ssa is true */
> + unsigned write_mask : NIR_MAX_VEC_COMPONENTS; /* ignored if
> dest.is_ssa is true */
>
Should this be nir_component_mask_t?
> } nir_alu_dest;
>
> typedef enum {
> @@ -841,14 +843,14 @@ typedef struct {
> /**
> * The number of components in each input
> */
> -
