[Mesa-dev] [PATCH 03/23] intel/eu: Use brw_set_desc() along with a helper to set common descriptor controls.
Kenneth Graunke
kenneth at whitecape.org
Thu Jun 21 20:51:12 UTC 2018
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);
> + }
> +}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180621/798cbdc4/attachment.sig>
More information about the mesa-dev
mailing list