[Mesa-dev] [PATCH 48.1/59] nir: Remove incorrect src_size vs dest_size assertions

[Connor Abbott]

On Wed, Oct 26, 2016 at 11:23 PM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> The bit-size of the sources and the bit-size of the destinations is
> almost always the same for instructions that can have variable sizes.
> The exception is shift instructions.  We can have shift instructions
> where the destination and the first source are (matching) 8-, 16-, 32-,
> or 64-bits.  However, in all these cases the second source is 32-bit.

This doesn't seem right. The assertion is just checking that unsized
sources & destinations match, which is definitely what we want -- we
don't want to shift a 32-bit value into a 64-bit value, for example.
Instead, we should be marking the shift amount as explicitly sized in
nir_opcodes.py, something like:

opcode("ishr", 0, tint, [0, 0], [tint, tuint32], "", "src0 >> src1")

and similar for the others. If that doesn't work, I could take a look
if you push what you tried to fdo.

>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: Connor Abbott <cwabbott0 at gmail.com>
> Cc: Jason Ekstrand <jason at jlekstrand.net>
> ---
>  src/compiler/nir/nir_builder.h  | 4 +---
>  src/compiler/nir/nir_validate.c | 7 -------
>  2 files changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/src/compiler/nir/nir_builder.h b/src/compiler/nir/nir_builder.h
> index 040f03e..e497a83 100644
> --- a/src/compiler/nir/nir_builder.h
> +++ b/src/compiler/nir/nir_builder.h
> @@ -204,9 +204,7 @@ nir_build_alu(nir_builder *build, nir_op op, nir_ssa_def *src0,
>        for (unsigned i = 0; i < op_info->num_inputs; i++) {
>           unsigned src_bit_size = instr->src[i].src.ssa->bit_size;
>           if (nir_alu_type_get_type_size(op_info->input_types[i]) == 0) {
> -            if (bit_size)
> -               assert(src_bit_size == bit_size);
> -            else
> +            if (bit_size == 0)
>                 bit_size = src_bit_size;
>           } else {
>              assert(src_bit_size ==
> diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c
> index bd73f04..9045a81 100644
> --- a/src/compiler/nir/nir_validate.c
> +++ b/src/compiler/nir/nir_validate.c
> @@ -238,13 +238,6 @@ validate_alu_src(nir_alu_instr *instr, unsigned index, validate_state *state)
>     if (nir_alu_type_get_type_size(src_type)) {
>        /* This source has an explicit bit size */
>        validate_assert(state, nir_alu_type_get_type_size(src_type) == src_bit_size);
> -   } else {
> -      if (!nir_alu_type_get_type_size(nir_op_infos[instr->op].output_type)) {
> -         unsigned dest_bit_size =
> -            instr->dest.dest.is_ssa ? instr->dest.dest.ssa.bit_size
> -                                    : instr->dest.dest.reg.reg->bit_size;
> -         validate_assert(state, dest_bit_size == src_bit_size);
> -      }
>     }
>
>     validate_src(&src->src, state);
> --
> 2.5.5
>