[Mesa-dev] [PATCH 5/7] i965: Abstract BRW_REGISTER_TYPE_* into an enum with unique values.

Kenneth Graunke kenneth at whitecape.org
Tue Dec 10 02:33:16 PST 2013


On released hardware, values 4-6 are overloaded.  For normal registers,
they mean UB/B/DF.  But for immediates, they mean UV/VF/V.

Previously, we just created #defines for each name, reusing the same
value.  This meant we could directly splat the brw_reg::type field into
the assembly encoding, which was fairly nice, and worked well.

Unfortunately, Broadwell makes this infeasible: the HF and DF types are
represented as different numeric values depending on whether the
source register is an immediate or not.

To preserve sanity, I decided to simply convert BRW_REGISTER_TYPE_* to
an abstract enum that has a unique value for each register type, and
write translation functions.  One nice benefit is that we can add
assertions about register files and generations.

I've chosen not to convert brw_reg::type to the enum, since converting
it caused a lot of trouble due to C++ enum rules (even though it's
defined in an extern "C" block...).

Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/mesa/drivers/dri/i965/brw_defines.h | 22 +++++++------
 src/mesa/drivers/dri/i965/brw_disasm.c  | 16 ++++-----
 src/mesa/drivers/dri/i965/brw_eu_emit.c | 57 ++++++++++++++++++++++++++++++---
 src/mesa/drivers/dri/i965/brw_reg.h     | 22 +++++++++++++
 4 files changed, 95 insertions(+), 22 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
index abecfc8..e6a3424 100644
--- a/src/mesa/drivers/dri/i965/brw_defines.h
+++ b/src/mesa/drivers/dri/i965/brw_defines.h
@@ -982,16 +982,18 @@ operator|(brw_urb_write_flags x, brw_urb_write_flags y)
 #define BRW_MESSAGE_REGISTER_FILE         2
 #define BRW_IMMEDIATE_VALUE               3
 
-#define BRW_REGISTER_TYPE_UD  0
-#define BRW_REGISTER_TYPE_D   1
-#define BRW_REGISTER_TYPE_UW  2
-#define BRW_REGISTER_TYPE_W   3
-#define BRW_REGISTER_TYPE_UB  4
-#define BRW_REGISTER_TYPE_B   5
-#define BRW_REGISTER_TYPE_UV  4	/* Gen6+ packed unsigned immediate vector */
-#define BRW_REGISTER_TYPE_VF  5	/* packed float vector, immediates only? */
-#define BRW_REGISTER_TYPE_V   6	/* packed int vector, immediates only, uword dest only */
-#define BRW_REGISTER_TYPE_F   7
+#define BRW_HW_REG_TYPE_UD  0
+#define BRW_HW_REG_TYPE_D   1
+#define BRW_HW_REG_TYPE_UW  2
+#define BRW_HW_REG_TYPE_W   3
+#define BRW_HW_REG_TYPE_F   7
+
+#define BRW_HW_REG_NON_IMM_TYPE_UB  4
+#define BRW_HW_REG_NON_IMM_TYPE_B   5
+
+#define BRW_HW_REG_IMM_TYPE_UV  4 /* Gen6+ packed unsigned immediate vector */
+#define BRW_HW_REG_IMM_TYPE_VF  5 /* packed float immediate vector */
+#define BRW_HW_REG_IMM_TYPE_V   6 /* packed int imm. vector; uword dest only */
 
 /* SNB adds 3-src instructions (MAD and LRP) that only operate on floats, so
  * the types were implied. IVB adds BFE and BFI2 that operate on doublewords
diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c b/src/mesa/drivers/dri/i965/brw_disasm.c
index 4334874..e5dcbb8 100644
--- a/src/mesa/drivers/dri/i965/brw_disasm.c
+++ b/src/mesa/drivers/dri/i965/brw_disasm.c
@@ -878,28 +878,28 @@ static int src2_3src (FILE *file, struct brw_instruction *inst)
 
 static int imm (FILE *file, unsigned type, struct brw_instruction *inst) {
     switch (type) {
-    case BRW_REGISTER_TYPE_UD:
+    case BRW_HW_REG_TYPE_UD:
 	format (file, "0x%08xUD", inst->bits3.ud);
 	break;
-    case BRW_REGISTER_TYPE_D:
+    case BRW_HW_REG_TYPE_D:
 	format (file, "%dD", inst->bits3.d);
 	break;
-    case BRW_REGISTER_TYPE_UW:
+    case BRW_HW_REG_TYPE_UW:
 	format (file, "0x%04xUW", (uint16_t) inst->bits3.ud);
 	break;
-    case BRW_REGISTER_TYPE_W:
+    case BRW_HW_REG_TYPE_W:
 	format (file, "%dW", (int16_t) inst->bits3.d);
 	break;
-    case BRW_REGISTER_TYPE_UV:
+    case BRW_HW_REG_IMM_TYPE_UV:
 	format (file, "0x%08xUV", inst->bits3.ud);
 	break;
-    case BRW_REGISTER_TYPE_VF:
+    case BRW_HW_REG_IMM_TYPE_VF:
 	format (file, "Vector Float");
 	break;
-    case BRW_REGISTER_TYPE_V:
+    case BRW_HW_REG_IMM_TYPE_V:
 	format (file, "0x%08xV", inst->bits3.ud);
 	break;
-    case BRW_REGISTER_TYPE_F:
+    case BRW_HW_REG_TYPE_F:
 	format (file, "%-gF", inst->bits3.f);
     }
     return 0;
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index 36bb3ce..b7a88931 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -99,6 +99,52 @@ gen7_convert_mrf_to_grf(struct brw_compile *p, struct brw_reg *reg)
    }
 }
 
+/**
+ * Convert a brw_reg_type enumeration value into the hardware representation.
+ *
+ * The hardware encoding may depend on whether the value is an immediate.
+ */
+unsigned
+brw_reg_type_to_hw_type(const struct brw_context *brw,
+                        enum brw_reg_type type, unsigned file)
+{
+   bool imm = file == BRW_IMMEDIATE_VALUE;
+
+   if (file == BRW_IMMEDIATE_VALUE) {
+      const static int imm_hw_types[] = {
+         [BRW_REGISTER_TYPE_UD] = BRW_HW_REG_TYPE_UD,
+         [BRW_REGISTER_TYPE_D]  = BRW_HW_REG_TYPE_D,
+         [BRW_REGISTER_TYPE_UW] = BRW_HW_REG_TYPE_UW,
+         [BRW_REGISTER_TYPE_W]  = BRW_HW_REG_TYPE_W,
+         [BRW_REGISTER_TYPE_F]  = BRW_HW_REG_TYPE_F,
+         [BRW_REGISTER_TYPE_UB] = -1,
+         [BRW_REGISTER_TYPE_B]  = -1,
+         [BRW_REGISTER_TYPE_UV] = BRW_HW_REG_IMM_TYPE_UV,
+         [BRW_REGISTER_TYPE_VF] = BRW_HW_REG_IMM_TYPE_VF,
+         [BRW_REGISTER_TYPE_V]  = BRW_HW_REG_IMM_TYPE_V,
+      };
+      assert(type < ARRAY_SIZE(imm_hw_types));
+      assert(imm_hw_types[type] != -1);
+      return imm_hw_types[type];
+   } else {
+      /* Non-immediate registers */
+      const static int hw_types[] = {
+         [BRW_REGISTER_TYPE_UD] = BRW_HW_REG_TYPE_UD,
+         [BRW_REGISTER_TYPE_D]  = BRW_HW_REG_TYPE_D,
+         [BRW_REGISTER_TYPE_UW] = BRW_HW_REG_TYPE_UW,
+         [BRW_REGISTER_TYPE_W]  = BRW_HW_REG_TYPE_W,
+         [BRW_REGISTER_TYPE_UB] = BRW_HW_REG_NON_IMM_TYPE_UB,
+         [BRW_REGISTER_TYPE_B]  = BRW_HW_REG_NON_IMM_TYPE_B,
+         [BRW_REGISTER_TYPE_F]  = BRW_HW_REG_TYPE_F,
+         [BRW_REGISTER_TYPE_UV] = -1,
+         [BRW_REGISTER_TYPE_VF] = -1,
+         [BRW_REGISTER_TYPE_V]  = -1,
+      };
+      assert(type < ARRAY_SIZE(hw_types));
+      assert(hw_types[type] != -1);
+      return hw_types[type];
+   }
+}
 
 void
 brw_set_dest(struct brw_compile *p, struct brw_instruction *insn,
@@ -111,7 +157,8 @@ brw_set_dest(struct brw_compile *p, struct brw_instruction *insn,
    gen7_convert_mrf_to_grf(p, &dest);
 
    insn->bits1.da1.dest_reg_file = dest.file;
-   insn->bits1.da1.dest_reg_type = dest.type;
+   insn->bits1.da1.dest_reg_type =
+      brw_reg_type_to_hw_type(p->brw, dest.type, dest.file);
    insn->bits1.da1.dest_address_mode = dest.address_mode;
 
    if (dest.address_mode == BRW_ADDRESS_DIRECT) {
@@ -264,7 +311,8 @@ brw_set_src0(struct brw_compile *p, struct brw_instruction *insn,
    validate_reg(insn, reg);
 
    insn->bits1.da1.src0_reg_file = reg.file;
-   insn->bits1.da1.src0_reg_type = reg.type;
+   insn->bits1.da1.src0_reg_type =
+      brw_reg_type_to_hw_type(brw, reg.type, reg.file);
    insn->bits2.da1.src0_abs = reg.abs;
    insn->bits2.da1.src0_negate = reg.negate;
    insn->bits2.da1.src0_address_mode = reg.address_mode;
@@ -275,7 +323,7 @@ brw_set_src0(struct brw_compile *p, struct brw_instruction *insn,
       /* Required to set some fields in src1 as well:
        */
       insn->bits1.da1.src1_reg_file = 0; /* arf */
-      insn->bits1.da1.src1_reg_type = reg.type;
+      insn->bits1.da1.src1_reg_type = insn->bits1.da1.src0_reg_type;
    }
    else
    {
@@ -345,7 +393,8 @@ void brw_set_src1(struct brw_compile *p,
    validate_reg(insn, reg);
 
    insn->bits1.da1.src1_reg_file = reg.file;
-   insn->bits1.da1.src1_reg_type = reg.type;
+   insn->bits1.da1.src1_reg_type =
+      brw_reg_type_to_hw_type(p->brw, reg.type, reg.file);
    insn->bits3.da1.src1_abs = reg.abs;
    insn->bits3.da1.src1_negate = reg.negate;
 
diff --git a/src/mesa/drivers/dri/i965/brw_reg.h b/src/mesa/drivers/dri/i965/brw_reg.h
index 6ffcd7b..8734958 100644
--- a/src/mesa/drivers/dri/i965/brw_reg.h
+++ b/src/mesa/drivers/dri/i965/brw_reg.h
@@ -88,6 +88,28 @@ brw_is_single_value_swizzle(int swiz)
            swiz == BRW_SWIZZLE_WWWW);
 }
 
+enum brw_reg_type {
+   BRW_REGISTER_TYPE_UD = 0,
+   BRW_REGISTER_TYPE_D,
+   BRW_REGISTER_TYPE_UW,
+   BRW_REGISTER_TYPE_W,
+   BRW_REGISTER_TYPE_F,
+
+   /** Non-immediates only: @{ */
+   BRW_REGISTER_TYPE_UB,
+   BRW_REGISTER_TYPE_B,
+   /** @} */
+
+   /** Immediates only: @{ */
+   BRW_REGISTER_TYPE_UV,
+   BRW_REGISTER_TYPE_V,
+   BRW_REGISTER_TYPE_VF,
+   /** @} */
+};
+
+unsigned brw_reg_type_to_hw_type(const struct brw_context *brw,
+                                 enum brw_reg_type type, unsigned file);
+
 #define REG_SIZE (8*4)
 
 /* These aren't hardware structs, just something useful for us to pass around:
-- 
1.8.4.4



More information about the mesa-dev mailing list