[Mesa-dev] [PATCH 18/18] i965: Validate "Special Cases for Byte Operations"

Matt Turner mattst88 at gmail.com
Tue Nov 22 19:59:52 UTC 2016


Do this in general_restrictions_based_on_operand_types() because the two
rules that "Special Cases for Byte Operations" relax are checked there.
---
 src/mesa/drivers/dri/i965/brw_eu_validate.c    | 70 +++++++++++++++++---
 src/mesa/drivers/dri/i965/test_eu_validate.cpp | 89 ++++++++++++++++++++++++++
 2 files changed, 150 insertions(+), 9 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_validate.c b/src/mesa/drivers/dri/i965/brw_eu_validate.c
index e9b55ed..7d800c1 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_validate.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_validate.c
@@ -78,6 +78,34 @@ inst_is_send(const struct gen_device_info *devinfo, const brw_inst *inst)
    }
 }
 
+static unsigned
+signed_type(unsigned type)
+{
+   switch (type) {
+   case BRW_HW_REG_TYPE_UD:         return BRW_HW_REG_TYPE_D;
+   case BRW_HW_REG_TYPE_UW:         return BRW_HW_REG_TYPE_W;
+   case BRW_HW_REG_NON_IMM_TYPE_UB: return BRW_HW_REG_NON_IMM_TYPE_B;
+   case GEN8_HW_REG_TYPE_UQ:        return GEN8_HW_REG_TYPE_Q;
+   default:                         return type;
+   }
+}
+
+static bool
+inst_is_raw_move(const struct gen_device_info *devinfo, const brw_inst *inst)
+{
+   unsigned dst_type = signed_type(brw_inst_dst_reg_type(devinfo, inst));
+   unsigned src_type = signed_type(brw_inst_src0_reg_type(devinfo, inst));
+
+   if (brw_inst_src0_reg_file(devinfo, inst) != BRW_IMMEDIATE_VALUE &&
+       (brw_inst_src0_negate(devinfo, inst) ||
+        brw_inst_src0_abs(devinfo, inst)))
+      return false;
+
+   return brw_inst_opcode(devinfo, inst) == BRW_OPCODE_MOV &&
+          brw_inst_saturate(devinfo, inst) == 0 &&
+          dst_type == src_type;
+}
+
 static bool
 dst_is_null(const struct gen_device_info *devinfo, const brw_inst *inst)
 {
@@ -417,10 +445,21 @@ general_restrictions_based_on_operand_types(const struct gen_device_info *devinf
    if (desc->ndst == 0)
       return (struct string){};
 
-   /* FINISHME: check special cases for byte operations */
-   if (brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_B ||
-       brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_UB)
-      return (struct string){};
+   unsigned dst_stride = 1 << (brw_inst_dst_hstride(devinfo, inst) - 1);
+   bool dst_type_is_byte =
+      brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_B ||
+      brw_inst_dst_reg_type(devinfo, inst) == BRW_HW_REG_NON_IMM_TYPE_UB;
+
+   if (dst_type_is_byte) {
+      if (is_packed(exec_size * dst_stride, exec_size, dst_stride)) {
+         if (!inst_is_raw_move(devinfo, inst)) {
+            ERROR("Only raw MOV supports a packed-byte destination");
+            return error_msg;
+         } else {
+            return (struct string){};
+         }
+      }
+   }
 
    unsigned exec_type = execution_type(devinfo, inst);
    unsigned exec_type_size =
@@ -428,17 +467,30 @@ general_restrictions_based_on_operand_types(const struct gen_device_info *devinf
    unsigned dst_type_size = brw_element_size(devinfo, inst, dst);
 
    if (exec_type_size > dst_type_size) {
-      unsigned dst_stride = 1 << (brw_inst_dst_hstride(devinfo, inst) - 1);
       ERROR_IF(dst_stride * dst_type_size != exec_type_size,
                "Destination stride must be equal to the ratio of the sizes of "
                "the execution data type to the destination type");
 
+      unsigned subreg = brw_inst_dst_da1_subreg_nr(devinfo, inst);
+
       if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1 &&
           brw_inst_dst_address_mode(devinfo, inst) == BRW_ADDRESS_DIRECT) {
-         unsigned subreg = brw_inst_dst_da1_subreg_nr(devinfo, inst);
-         ERROR_IF(subreg % exec_type_size != 0,
-                  "Destination subreg must be aligned to the size of the "
-                  "execution data type");
+         /* The i965 PRM says:
+          *
+          *    Implementation Restriction: The relaxed alignment rule for byte
+          *    destination (#10.5) is not supported.
+          */
+         if ((devinfo->gen > 4 || devinfo->is_g4x) && dst_type_is_byte) {
+            ERROR_IF(subreg % exec_type_size != 0 &&
+                     subreg % exec_type_size != 1,
+                     "Destination subreg must be aligned to the size of the "
+                     "execution data type (or to the next lowest byte for byte "
+                     "destinations)");
+         } else {
+            ERROR_IF(subreg % exec_type_size != 0,
+                     "Destination subreg must be aligned to the size of the "
+                     "execution data type");
+         }
       }
    }
 
diff --git a/src/mesa/drivers/dri/i965/test_eu_validate.cpp b/src/mesa/drivers/dri/i965/test_eu_validate.cpp
index f4d0e94..3e045f3 100644
--- a/src/mesa/drivers/dri/i965/test_eu_validate.cpp
+++ b/src/mesa/drivers/dri/i965/test_eu_validate.cpp
@@ -756,3 +756,92 @@ TEST_P(validation_test, one_src_two_dst)
       EXPECT_FALSE(validate(p));
    }
 }
+
+TEST_P(validation_test, packed_byte_destination)
+{
+   static const struct {
+      enum brw_reg_type dst_type;
+      enum brw_reg_type src_type;
+      bool neg, abs, sat;
+      bool expected_result;
+   } move[] = {
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UB, 0, 0, 0, true },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_B , 0, 0, 0, true },
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_B , 0, 0, 0, true },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_UB, 0, 0, 0, true },
+
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UB, 1, 0, 0, false },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_B , 1, 0, 0, false },
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_B , 1, 0, 0, false },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_UB, 1, 0, 0, false },
+
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UB, 0, 1, 0, false },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_B , 0, 1, 0, false },
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_B , 0, 1, 0, false },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_UB, 0, 1, 0, false },
+
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UB, 0, 0, 1, false },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_B , 0, 0, 1, false },
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_B , 0, 0, 1, false },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_UB, 0, 0, 1, false },
+
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UW, 0, 0, 0, false },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_W , 0, 0, 0, false },
+      { BRW_REGISTER_TYPE_UB, BRW_REGISTER_TYPE_UD, 0, 0, 0, false },
+      { BRW_REGISTER_TYPE_B , BRW_REGISTER_TYPE_D , 0, 0, 0, false },
+   };
+
+   for (unsigned i = 0; i < sizeof(move) / sizeof(move[0]); i++) {
+      brw_MOV(p, retype(g0, move[i].dst_type), retype(g0, move[i].src_type));
+      brw_inst_set_src0_negate(&devinfo, last_inst, move[i].neg);
+      brw_inst_set_src0_abs(&devinfo, last_inst, move[i].abs);
+      brw_inst_set_saturate(&devinfo, last_inst, move[i].sat);
+
+      EXPECT_EQ(move[i].expected_result, validate(p));
+
+      clear_instructions(p);
+   }
+
+   brw_SEL(p, retype(g0, BRW_REGISTER_TYPE_UB),
+              retype(g0, BRW_REGISTER_TYPE_UB),
+              retype(g0, BRW_REGISTER_TYPE_UB));
+   brw_inst_set_pred_control(&devinfo, last_inst, BRW_PREDICATE_NORMAL);
+
+   EXPECT_FALSE(validate(p));
+
+   clear_instructions(p);
+
+   brw_SEL(p, retype(g0, BRW_REGISTER_TYPE_B),
+              retype(g0, BRW_REGISTER_TYPE_B),
+              retype(g0, BRW_REGISTER_TYPE_B));
+   brw_inst_set_pred_control(&devinfo, last_inst, BRW_PREDICATE_NORMAL);
+
+   EXPECT_FALSE(validate(p));
+}
+
+TEST_P(validation_test, byte_destination_relaxed_alignment)
+{
+   brw_SEL(p, retype(g0, BRW_REGISTER_TYPE_B),
+              retype(g0, BRW_REGISTER_TYPE_W),
+              retype(g0, BRW_REGISTER_TYPE_W));
+   brw_inst_set_pred_control(&devinfo, last_inst, BRW_PREDICATE_NORMAL);
+   brw_inst_set_dst_hstride(&devinfo, last_inst, BRW_HORIZONTAL_STRIDE_2);
+
+   EXPECT_TRUE(validate(p));
+
+   clear_instructions(p);
+
+   brw_SEL(p, retype(g0, BRW_REGISTER_TYPE_B),
+              retype(g0, BRW_REGISTER_TYPE_W),
+              retype(g0, BRW_REGISTER_TYPE_W));
+   brw_inst_set_pred_control(&devinfo, last_inst, BRW_PREDICATE_NORMAL);
+   brw_inst_set_dst_hstride(&devinfo, last_inst, BRW_HORIZONTAL_STRIDE_2);
+   brw_inst_set_dst_da1_subreg_nr(&devinfo, last_inst, 1);
+
+   if (devinfo.gen > 4 || devinfo.is_g4x) {
+      EXPECT_TRUE(validate(p));
+   } else {
+      EXPECT_FALSE(validate(p));
+   }
+
+}
-- 
2.7.3



More information about the mesa-dev mailing list