Hi curro,
Am 24.05.2016 um 09:18 schrieb Francisco Jerez:
> This implements some simple helper functions that can be used to
> specify the group of channel enable signals and compression enable
> that apply to a brw_inst instruction.
>
> It's intended to replace brw_set_default_compression_control
> eventually because the current interface has a number of shortcomings
> inherited from the Gen-4-5-centric representation of compression and
> group controls as a single non-orthogonal enum: On the one hand it
> doesn't work for specifying arbitrary group controls other than 1Q and
> 2Q, which are frequently useful in SIMD32 and FP64 programs. On the
> other hand the current interface forces you to update the compression
> *and* group controls simultaneously, which has been the source of a
> number of generator bugs (a bunch of them fixed in this series),
> because in many cases we would end up resetting the group controls to
> zero inadvertently even though everything we wanted to do was disable
> instruction compression -- The latter seems especially unfortunate on
> Gen6+ hardware which have no explicit compression control, so we would
> end up bashing the quarter control field of the instruction for no
> benefit.
>
> Instead of a single function that updates both at the same time
> introduce separate interfaces to update one or the other independently
> preserving the current value of the other (which typically comes from
> the back-end IR so it has to be respected).
> ---
> src/mesa/drivers/dri/i965/brw_eu.c | 69
> ++++++++++++++++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/brw_eu.h | 6 ++++
> 2 files changed, 75 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.c
> b/src/mesa/drivers/dri/i965/brw_eu.c
> index 48c8439..f1161d2 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu.c
> @@ -218,6 +218,75 @@ brw_set_default_compression_control(struct brw_codegen
> *p,
> }
> }
>
> +/**
> + * Enable or disable instruction compression on the given instruction leaving
> + * the currently selected channel enable group untouched.
> + */
> +void
> +brw_inst_set_compression(const struct brw_device_info *devinfo,
> + brw_inst *inst, bool on)
> +{
> + if (devinfo->gen >= 6) {
> + /* No-op, the EU will figure out for us whether the instruction needs
> to
> + * be compressed.
> + */
> + } else {
> + /* The channel group and compression controls are non-orthogonal, there
> + * are two possible representations for uncompressed instructions and
> we
> + * may need to preserve the current one to avoid changing the selected
> + * channel group inadvertently.
> + */
> + if (on)
> + brw_inst_set_qtr_control(devinfo, inst, BRW_COMPRESSION_COMPRESSED);
> + else if (brw_inst_qtr_control(devinfo, inst)
> + == BRW_COMPRESSION_COMPRESSED)
> + brw_inst_set_qtr_control(devinfo, inst, BRW_COMPRESSION_NONE);
> + }
> +}
> +
> +void
> +brw_set_default_compression(struct brw_codegen *p, bool on)
> +{
> + brw_inst_set_compression(p->devinfo, p->current, on);
> +}
> +
> +/**
> + * Apply the range of channel enable signals given by
> + * [group, group + exec_size[ to the instruction passed as argument.
> + */
> +void
> +brw_inst_set_group(const struct brw_device_info *devinfo,
> + brw_inst *inst, unsigned group)
> +{
> + if (devinfo->gen >= 7) {
> + assert(group % 4 == 0 && group < 32);
> + brw_inst_set_qtr_control(devinfo, inst, group / 8);
> + brw_inst_set_nib_control(devinfo, inst, group / 4 % 2);
> +
> + } else if (devinfo->gen >= 6) {
Could you make that ==6, so the two conditions do not overlap?
--Michael
> + assert(group % 8 == 0 && group < 32);
> + brw_inst_set_qtr_control(devinfo, inst, group / 8);
> +
> + } else {
> + assert(group % 8 == 0 && group < 16);
> + /* The channel group and compression controls are non-orthogonal, there
> + * are two possible representations for group zero and we may need to
> + * preserve the current one to avoid changing the selected compression
> + * enable inadvertently.
> + */
> + if (group == 8)
> + brw_inst_set_qtr_control(devinfo, inst, BRW_COMPRESSION_2NDHALF);
> + else if (brw_inst_qtr_control(devinfo, inst) ==
> BRW_COMPRESSION_2NDHALF)
> + brw_inst_set_qtr_control(devinfo, inst, BRW_COMPRESSION_NONE);
> + }
> +}
> +
> +void
> +brw_set_default_group(struct brw_codegen *p, unsigned group)
> +{
> + brw_inst_set_group(p->devinfo, p->current, group);
> +}
> +
> void brw_set_default_mask_control( struct brw_codegen *p, unsigned value )
> {
> brw_inst_set_mask_control(p->devinfo, p->current, value);
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
> index bea90f4..03400ae 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -101,6 +101,12 @@ void brw_set_default_exec_size(struct brw_codegen *p,
> unsigned value);
> void brw_set_default_mask_control( struct brw_codegen *p, unsigned value );
> void brw_set_default_saturate( struct brw_codegen *p, bool enable );
> void brw_set_default_access_mode( struct brw_codegen *p, unsigned
> access_mode );
> +void brw_inst_set_compression(const struct brw_device_info *devinfo,
> + brw_inst *inst, bool on);
> +void brw_set_default_compression(struct brw_codegen *p, bool on);
> +void brw_inst_set_group(const struct brw_device_info *devinfo,
> + brw_inst *inst, unsigned group);
> +void brw_set_default_group(struct brw_codegen *p, unsigned group);
> void brw_set_default_compression_control(struct brw_codegen *p, enum
> brw_compression c);
> void brw_set_default_predicate_control( struct brw_codegen *p, unsigned pc );
> void brw_set_default_predicate_inverse(struct brw_codegen *p, bool
> predicate_inverse);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
