[Mesa-dev] [PATCH] i965: expose BRW_OPCODE_[F32TO16/F16TO32] opcode_descs on gen8+

[Matt Turner]

On Wed, Mar 29, 2017 at 4:47 AM, Alejandro Piñeiro <apinheiro at igalia.com> wrote:
> Technically those hw operations are only available on gen7, as gen8+
> support the conversion on the MOV. But, when using the builder to
> implement nir operations (example: nir_op_fquantize2f16), it is not
> needed to do the gen check. This check is done later, on the final
> emission at brw_F32TO16 (brw_eu_emit), choosing between the MOV or the
> specific operation accordingly.
>
> So in the middle, during optimization phases those hw operations can
> be around for gen8+ too.
>
> Without this patch, several (at least 95) vulkan-cts quantize tests
> crashes when using INTEL_DEBUG=optimizer. For example:
> dEQP-VK.spirv_assembly.instruction.graphics.opquantize.too_small_vert
> ---
>  src/intel/compiler/brw_eu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c
> index 77400c1..bff37d7 100644
> --- a/src/intel/compiler/brw_eu.c
> +++ b/src/intel/compiler/brw_eu.c
> @@ -499,10 +499,10 @@ static const struct opcode_desc opcode_descs[128] = {
>        .name = "csel",    .nsrc = 3, .ndst = 1, .gens = GEN_GE(GEN8),
>     },
>     [BRW_OPCODE_F32TO16] = {
> -      .name = "f32to16", .nsrc = 1, .ndst = 1, .gens = GEN7 | GEN75,
> +      .name = "f32to16", .nsrc = 1, .ndst = 1, .gens = GEN7 | GEN75 | GEN8 | GEN9,
>     },
>     [BRW_OPCODE_F16TO32] = {
> -      .name = "f16to32", .nsrc = 1, .ndst = 1, .gens = GEN7 | GEN75,
> +      .name = "f16to32", .nsrc = 1, .ndst = 1, .gens = GEN7 | GEN75 | GEN8 | GEN9,
>     },

This table is for hardware information, used by brw_eu_validate.c.
Since these opcodes do not exist on Gen8+, we should not add that to
the table.

I assume that the crashes you are referring to are assertion failures
in brw_instruction_name() -- assert(brw_opcode_desc(devinfo,
op)->name)

If that's the case, there's an identical case immediately above. We
use BRW_OPCODE_DO in the backend IRs, but that opcode is not used on
Gen6+. I would add two more cases for f32to16 and f16to32 there.

Perhaps we should not use BRW_OPCODE_* for operations used in the
backend IR that may not actually exist as a real opcode in hardware.
Not sure.