Skip to content

intel/brw: Decode instruction at one place during EU validation

Caio Oliveira requested to merge cmarcelo/mesa:brw-decoded-inst-for-eu-validation into main

On top of !31294 (merged).

The EU validation operates directly with the raw brw_inst, which is just actual bits that represent the EU instruction, using the various brw_inst_* helpers. For the same "logical" field (e.g. register file or register number), different instructions require different helpers because the bits are laid out differently for them.

There are two problems with this scheme:

  • The restrictions are documented in terms of the "logical" fields, which may require handling at the restriction code the various possible encodings. The decoding information gets spread into the various restrictions.
  • As a consequence, there are some fields that get decoded multiple times for the same instruction.

This MR proposes adding a struct to hold this decoded information. In the long run we may want to collapse this back into using fs_inst. For now it is practical to not do it. The new struct has only the fields necessary for EU validation. Some less used fields were not included. For those, validation code falls back to use brw_inst_* helpers using the raw instruction (also part of the struct).

To illustrate the general idea, this chunk of ADD3 validation

      for (unsigned i = 0; i < 3; i++) {
         enum brw_reg_type src_type;

         switch (i) {
         case 0: src_type = brw_inst_3src_a1_src0_type(devinfo, inst); break;
         case 1: src_type = brw_inst_3src_a1_src1_type(devinfo, inst); break;
         case 2: src_type = brw_inst_3src_a1_src2_type(devinfo, inst); break;
         default: unreachable("invalid src");
         }

         ERROR_IF(src_type != BRW_TYPE_D &&
                  src_type != BRW_TYPE_UD &&
                  src_type != BRW_TYPE_W &&
                  src_type != BRW_TYPE_UW,
                  "Source must be integer D, UD, W, or UW type.");

         if (i == 0) {
            if (brw_inst_3src_a1_src0_is_imm(devinfo, inst)) {
               ERROR_IF(src_type != BRW_TYPE_W &&
                        src_type != BRW_TYPE_UW,
                        "Immediate source must be integer W or UW type.");
            }
         } else if (i == 2) {
            if (brw_inst_3src_a1_src2_is_imm(devinfo, inst)) {
               ERROR_IF(src_type != BRW_TYPE_W &&
                        src_type != BRW_TYPE_UW,
                        "Immediate source must be integer W or UW type.");
            }
         }
      }

becomes

      for (unsigned i = 0; i < 3; i++) {
         enum brw_reg_type src_type = inst->src[i].type;

         ERROR_IF(src_type != BRW_TYPE_D &&
                  src_type != BRW_TYPE_UD &&
                  src_type != BRW_TYPE_W &&
                  src_type != BRW_TYPE_UW,
                  "Source must be integer D, UD, W, or UW type.");

         ERROR_IF(inst->src[i].file == IMM &&
                  src_type != BRW_TYPE_W &&
                  src_type != BRW_TYPE_UW,
                  "Immediate source must be integer W or UW type.");
      }

Another interesting example is that the sources are stored in an array, the various cases where we use a DO_SRC macro to call helpers for src0 and src1 can now be replaced by indexing on that array without macros.

Edited by Caio Oliveira

Merge request reports