On Monday, June 11, 2018 7:25:55 PM PDT Francisco Jerez wrote: > 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(). > --- > src/intel/compiler/brw_eu.h | 29 +++++--- > src/intel/compiler/brw_eu_emit.c | 108 > +++++++++++------------------- > src/intel/compiler/brw_vec4_generator.cpp | 17 +++-- > 3 files changed, 68 insertions(+), 86 deletions(-) > > diff --git a/src/intel/compiler/brw_eu.h b/src/intel/compiler/brw_eu.h > index 5a396339fde..b2b20713e45 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) > +{
Perhaps it would be good to add assert(msg_length >= 1 && msg_length <= 15); > + if (devinfo->gen >= 5) { assert(response_length <= 16); > + return (msg_length << 25 | > + response_length << 20 | > + header_present << 19); > + } else { assert(response_length <= 8); I'm not so concerned with validating the values here, just thinking it might make sense to verify that mlen fits in a U4, for example, so we don't accidentally bleed over into other fields when encoding it. I suppose the validator already catches this, though... > + return (msg_length << 20 | > + response_length << 16); > + } > +}
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev