intel/eu: Use brw_set_desc() along with a helper to set common descriptor controls.

This replaces brw_set_message_descriptor() with the composition of
brw_set_desc() and a new inline helper function that packs the common
message descriptor controls into an integer.  The goal is to represent
all message descriptors as a 32-bit integer which is written at once
into the instruction, which is more flexible (SENDS anyone?), robust
(see d2eecf0b0b24d203d0f171807681dffd830d54de fixing an issue
ultimately caused by some bits of the extended message descriptor
being left undefined) and future-proof than the current approach of
specifying the individual descriptor fields directly into the
instruction.

This approach also seems more self-documenting, since it will allow
removing calls to functions with way too many arguments like
brw_set_*_message() and brw_send_indirect_message(), and instead
provide a single descriptor argument constructed from an appropriate
combination of brw_*_desc() helpers.

Note that because brw_set_message_descriptor() was (conditionally?)
overriding fields of the instruction which strictly speaking weren't
part of the message descriptor, this involves calling
brw_inst_set_sfid() and brw_inst_set_eot() in some cases in addition
to brw_set_desc().

v2: Use SET_BITS macro instead of left shift (Ken).

Reviewed-by: Kenneth Graunke <[email protected]>

3 files changed, 68 insertions, 86 deletions

diff --git a/src/intel/compiler/brw_eu.h b/src/intel/compiler/brw_eu.h
index 30c987942c4..d69bc5526cd 100644
--- a/src/intel/compiler/brw_eu.h
+++ b/src/intel/compiler/brw_eu.h
@@ -256,14 +256,6 @@ void brw_set_sampler_message(struct brw_codegen *p,
                              unsigned simd_mode,
                              unsigned return_format);
 
-void brw_set_message_descriptor(struct brw_codegen *p,
-                                brw_inst *inst,
-                                enum brw_message_target sfid,
-                                unsigned msg_length,
-                                unsigned response_length,
-                                bool header_present,
-                                bool end_of_thread);
-
 void brw_set_dp_read_message(struct brw_codegen *p,
 			     brw_inst *insn,
 			     unsigned binding_table_index,
@@ -287,6 +279,27 @@ void brw_set_dp_write_message(struct brw_codegen *p,
 			      unsigned end_of_thread,
 			      unsigned send_commit_msg);
 
+/**
+ * Construct a message descriptor immediate with the specified common
+ * descriptor controls.
+ */
+static inline uint32_t
+brw_message_desc(const struct gen_device_info *devinfo,
+                 unsigned msg_length,
+                 unsigned response_length,
+                 bool header_present)
+{
+   if (devinfo->gen >= 5) {
+      return (SET_BITS(msg_length, 28, 25) |
+              SET_BITS(response_length, 24, 20) |
+              SET_BITS(header_present, 19, 19));
+   } else {
+      return (SET_BITS(msg_length, 23, 20) |
+              SET_BITS(response_length, 19, 16));
+   }
+}
+
+
 void brw_urb_WRITE(struct brw_codegen *p,
 		   struct brw_reg dest,
 		   unsigned msg_reg_nr,
diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c
index 5ac1498f0fc..aa405eb5501 100644
--- a/src/intel/compiler/brw_eu_emit.c
+++ b/src/intel/compiler/brw_eu_emit.c
@@ -360,48 +360,6 @@ brw_set_src1(struct brw_codegen *p, brw_inst *inst, struct brw_reg reg)
 }
 
 /**
- * Set the Message Descriptor and Extended Message Descriptor fields
- * for SEND messages.
- *
- * \note This zeroes out the Function Control bits, so it must be called
- *       \b before filling out any message-specific data.  Callers can
- *       choose not to fill in irrelevant bits; they will be zero.
- */
-void
-brw_set_message_descriptor(struct brw_codegen *p,
-			   brw_inst *inst,
-			   enum brw_message_target sfid,
-			   unsigned msg_length,
-			   unsigned response_length,
-			   bool header_present,
-			   bool end_of_thread)
-{
-   const struct gen_device_info *devinfo = p->devinfo;
-
-   brw_set_src1(p, inst, brw_imm_d(0));
-
-   /* For indirect sends, `inst` will not be the SEND/SENDC instruction
-    * itself; instead, it will be a MOV/OR into the address register.
-    *
-    * In this case, we avoid setting the extended message descriptor bits,
-    * since they go on the later SEND/SENDC instead and if set here would
-    * instead clobber the conditionalmod bits.
-    */
-   unsigned opcode = brw_inst_opcode(devinfo, inst);
-   if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC) {
-      brw_inst_set_sfid(devinfo, inst, sfid);
-   }
-
-   brw_inst_set_mlen(devinfo, inst, msg_length);
-   brw_inst_set_rlen(devinfo, inst, response_length);
-   brw_inst_set_eot(devinfo, inst, end_of_thread);
-
-   if (devinfo->gen >= 5) {
-      brw_inst_set_header_present(devinfo, inst, header_present);
-   }
-}
-
-/**
  * Specify the descriptor and extended descriptor immediate for a SEND(C)
  * message instruction.
  */
@@ -453,9 +411,10 @@ static void brw_set_math_message( struct brw_codegen *p,
       break;
    }
 
+   brw_set_desc(p, inst, brw_message_desc(
+                   devinfo, msg_length, response_length, false));
 
-   brw_set_message_descriptor(p, inst, BRW_SFID_MATH,
-			      msg_length, response_length, false, false);
+   brw_inst_set_sfid(devinfo, inst, BRW_SFID_MATH);
    brw_inst_set_math_msg_function(devinfo, inst, function);
    brw_inst_set_math_msg_signed_int(devinfo, inst, integer_type);
    brw_inst_set_math_msg_precision(devinfo, inst, low_precision);
@@ -473,8 +432,11 @@ static void brw_set_ff_sync_message(struct brw_codegen *p,
 {
    const struct gen_device_info *devinfo = p->devinfo;
 
-   brw_set_message_descriptor(p, insn, BRW_SFID_URB,
-			      1, response_length, true, end_of_thread);
+   brw_set_desc(p, insn, brw_message_desc(
+                   devinfo, 1, response_length, true));
+
+   brw_inst_set_sfid(devinfo, insn, BRW_SFID_URB);
+   brw_inst_set_eot(devinfo, insn, end_of_thread);
    brw_inst_set_urb_opcode(devinfo, insn, 1); /* FF_SYNC */
    brw_inst_set_urb_allocate(devinfo, insn, allocate);
    /* The following fields are not used by FF_SYNC: */
@@ -498,9 +460,11 @@ static void brw_set_urb_message( struct brw_codegen *p,
    assert(devinfo->gen < 7 || !(flags & BRW_URB_WRITE_ALLOCATE));
    assert(devinfo->gen >= 7 || !(flags & BRW_URB_WRITE_PER_SLOT_OFFSET));
 
-   brw_set_message_descriptor(p, insn, BRW_SFID_URB,
-			      msg_length, response_length, true,
-                              flags & BRW_URB_WRITE_EOT);
+   brw_set_desc(p, insn, brw_message_desc(
+                   devinfo, msg_length, response_length, true));
+
+   brw_inst_set_sfid(devinfo, insn, BRW_SFID_URB);
+   brw_inst_set_eot(devinfo, insn, !!(flags & BRW_URB_WRITE_EOT));
 
    if (flags & BRW_URB_WRITE_OWORD) {
       assert(msg_length == 2); /* header + one OWORD of data */
@@ -543,9 +507,11 @@ brw_set_dp_write_message(struct brw_codegen *p,
    const unsigned sfid = (devinfo->gen >= 6 ? target_cache :
                           BRW_SFID_DATAPORT_WRITE);
 
-   brw_set_message_descriptor(p, insn, sfid, msg_length, response_length,
-			      header_present, end_of_thread);
+   brw_set_desc(p, insn, brw_message_desc(
+                   devinfo, msg_length, response_length, header_present));
 
+   brw_inst_set_sfid(devinfo, insn, sfid);
+   brw_inst_set_eot(devinfo, insn, !!end_of_thread);
    brw_inst_set_binding_table_index(devinfo, insn, binding_table_index);
    brw_inst_set_dp_write_msg_type(devinfo, insn, msg_type);
    brw_inst_set_dp_write_msg_control(devinfo, insn, msg_control);
@@ -573,9 +539,12 @@ brw_set_dp_read_message(struct brw_codegen *p,
    const unsigned sfid = (devinfo->gen >= 6 ? target_cache :
                           BRW_SFID_DATAPORT_READ);
 
-   brw_set_message_descriptor(p, insn, sfid, msg_length, response_length,
-			      header_present, false);
+   brw_set_desc(p, insn, brw_message_desc(
+                   devinfo, msg_length, response_length, header_present));
 
+   const unsigned opcode = brw_inst_opcode(devinfo, insn);
+   if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC)
+      brw_inst_set_sfid(devinfo, insn, sfid);
    brw_inst_set_binding_table_index(devinfo, insn, binding_table_index);
    brw_inst_set_dp_read_msg_type(devinfo, insn, msg_type);
    brw_inst_set_dp_read_msg_control(devinfo, insn, msg_control);
@@ -597,9 +566,12 @@ brw_set_sampler_message(struct brw_codegen *p,
 {
    const struct gen_device_info *devinfo = p->devinfo;
 
-   brw_set_message_descriptor(p, inst, BRW_SFID_SAMPLER, msg_length,
-			      response_length, header_present, false);
+   brw_set_desc(p, inst, brw_message_desc(
+                   devinfo, msg_length, response_length, header_present));
 
+   const unsigned opcode = brw_inst_opcode(devinfo, inst);
+   if (opcode == BRW_OPCODE_SEND || opcode == BRW_OPCODE_SENDC)
+      brw_inst_set_sfid(devinfo, inst, BRW_SFID_SAMPLER);
    brw_inst_set_binding_table_index(devinfo, inst, binding_table_index);
    brw_inst_set_sampler(devinfo, inst, sampler);
    brw_inst_set_sampler_msg_type(devinfo, inst, msg_type);
@@ -628,8 +600,10 @@ gen7_set_dp_scratch_message(struct brw_codegen *p,
    const unsigned block_size = (devinfo->gen >= 8 ? _mesa_logbase2(num_regs) :
                                 num_regs - 1);
 
-   brw_set_message_descriptor(p, inst, GEN7_SFID_DATAPORT_DATA_CACHE,
-                              mlen, rlen, header_present, false);
+   brw_set_desc(p, inst, brw_message_desc(
+                   devinfo, mlen, rlen, header_present));
+
+   brw_inst_set_sfid(devinfo, inst, GEN7_SFID_DATAPORT_DATA_CACHE);
    brw_inst_set_dp_category(devinfo, inst, 1); /* Scratch Block Read/Write msgs */
    brw_inst_set_scratch_read_write(devinfo, inst, write);
    brw_inst_set_scratch_type(devinfo, inst, dword);
@@ -3309,11 +3283,10 @@ brw_set_memory_fence_message(struct brw_codegen *p,
 {
    const struct gen_device_info *devinfo = p->devinfo;
 
-   brw_set_message_descriptor(p, insn, sfid,
-                              1 /* message length */,
-                              (commit_enable ? 1 : 0) /* response length */,
-                              true /* header present */,
-                              false);
+   brw_set_desc(p, insn, brw_message_desc(
+                   devinfo, 1, (commit_enable ? 1 : 0), true));
+
+   brw_inst_set_sfid(devinfo, insn, sfid);
 
    switch (sfid) {
    case GEN6_SFID_DATAPORT_RENDER_CACHE:
@@ -3682,7 +3655,8 @@ void brw_shader_time_add(struct brw_codegen *p,
    brw_set_src0(p, send, brw_vec1_reg(payload.file,
                                       payload.nr, 0));
    brw_set_src1(p, send, brw_imm_ud(0));
-   brw_set_message_descriptor(p, send, sfid, 2, 0, false, false);
+   brw_set_desc(p, send, brw_message_desc(p->devinfo, 2, 0, false));
+   brw_inst_set_sfid(p->devinfo, send, sfid);
    brw_inst_set_binding_table_index(p->devinfo, send, surf_index);
    brw_set_dp_untyped_atomic_message(p, send, BRW_AOP_ADD, false);
 
@@ -3707,13 +3681,9 @@ brw_barrier(struct brw_codegen *p, struct brw_reg src)
    brw_set_dest(p, inst, retype(brw_null_reg(), BRW_REGISTER_TYPE_UW));
    brw_set_src0(p, inst, src);
    brw_set_src1(p, inst, brw_null_reg());
+   brw_set_desc(p, inst, brw_message_desc(devinfo, 1, 0, false));
 
-   brw_set_message_descriptor(p, inst, BRW_SFID_MESSAGE_GATEWAY,
-                              1 /* msg_length */,
-                              0 /* response_length */,
-                              false /* header_present */,
-                              false /* end_of_thread */);
-
+   brw_inst_set_sfid(devinfo, inst, BRW_SFID_MESSAGE_GATEWAY);
    brw_inst_set_gateway_notify(devinfo, inst, 1);
    brw_inst_set_gateway_subfuncid(devinfo, inst,
                                   BRW_MESSAGE_GATEWAY_SFID_BARRIER_MSG);
diff --git a/src/intel/compiler/brw_vec4_generator.cpp b/src/intel/compiler/brw_vec4_generator.cpp
index 7519ccc9df3..386b071cfda 100644
--- a/src/intel/compiler/brw_vec4_generator.cpp
+++ b/src/intel/compiler/brw_vec4_generator.cpp
@@ -777,10 +777,9 @@ generate_tcs_urb_write(struct brw_codegen *p,
    brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND);
    brw_set_dest(p, send, brw_null_reg());
    brw_set_src0(p, send, urb_header);
+   brw_set_desc(p, send, brw_message_desc(devinfo, inst->mlen, 0, true));
 
-   brw_set_message_descriptor(p, send, BRW_SFID_URB,
-                              inst->mlen /* mlen */, 0 /* rlen */,
-                              true /* header */, false /* eot */);
+   brw_inst_set_sfid(devinfo, send, BRW_SFID_URB);
    brw_inst_set_urb_opcode(devinfo, send, BRW_URB_OPCODE_WRITE_OWORD);
    brw_inst_set_urb_global_offset(devinfo, send, inst->offset);
    if (inst->urb_write_flags & BRW_URB_WRITE_EOT) {
@@ -953,9 +952,9 @@ generate_vec4_urb_read(struct brw_codegen *p,
    brw_set_dest(p, send, dst);
    brw_set_src0(p, send, header);
 
-   brw_set_message_descriptor(p, send, BRW_SFID_URB,
-                              1 /* mlen */, 1 /* rlen */,
-                              true /* header */, false /* eot */);
+   brw_set_desc(p, send, brw_message_desc(devinfo, 1, 1, true));
+
+   brw_inst_set_sfid(devinfo, send, BRW_SFID_URB);
    brw_inst_set_urb_opcode(devinfo, send, BRW_URB_OPCODE_READ_OWORD);
    brw_inst_set_urb_swizzle_control(devinfo, send, BRW_URB_SWIZZLE_INTERLEAVE);
    brw_inst_set_urb_per_slot_offset(devinfo, send, 1);
@@ -989,9 +988,9 @@ generate_tcs_release_input(struct brw_codegen *p,
    brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND);
    brw_set_dest(p, send, brw_null_reg());
    brw_set_src0(p, send, header);
-   brw_set_message_descriptor(p, send, BRW_SFID_URB,
-                              1 /* mlen */, 0 /* rlen */,
-                              true /* header */, false /* eot */);
+   brw_set_desc(p, send, brw_message_desc(devinfo, 1, 0, true));
+
+   brw_inst_set_sfid(devinfo, send, BRW_SFID_URB);
    brw_inst_set_urb_opcode(devinfo, send, BRW_URB_OPCODE_READ_OWORD);
    brw_inst_set_urb_complete(devinfo, send, 1);
    brw_inst_set_urb_swizzle_control(devinfo, send, is_unpaired.ud ?