[Mesa-dev] [PATCH 41/95] i965/vec4: make the generator set correct NibCtrl for SIMD4 DF instructions

[Francisco Jerez]

Iago Toral Quiroga <itoral at igalia.com> writes:

> From the HSW PRM, Command Reference, QtrCtrl:
>
>    "NibCtrl is only allowed for SIMD4 instructions with a DF (Double Float)
>     source or destination type."
>
> v2 (Samuel): Assert that the type is DF.
>
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index c6e040e..dc4f6db 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -1494,6 +1494,7 @@ generate_code(struct brw_codegen *p,
>        brw_set_default_saturate(p, inst->saturate);
>        brw_set_default_mask_control(p, inst->force_writemask_all);
>        brw_set_default_acc_write_control(p, inst->writes_accumulator);
> +      brw_set_default_group(p, 0);
>
>        assert(inst->base_mrf + inst->mlen <= BRW_MAX_MRF(devinfo->gen));
>        assert(inst->mlen <= BRW_MAX_MSG_LENGTH);
> @@ -1529,6 +1530,14 @@ generate_code(struct brw_codegen *p,
>        } else {
>           assert(inst->exec_size == 8 || inst->exec_size == 4);
>           brw_set_default_exec_size(p, cvt(inst->exec_size) - 1);
> +         if (inst->exec_size == 4 && !inst->force_writemask_all) {
> +            assert(inst->dst.type == BRW_REGISTER_TYPE_DF ||
> +                   inst->src[0].type == BRW_REGISTER_TYPE_DF ||
> +                   inst->src[1].type == BRW_REGISTER_TYPE_DF ||
> +                   inst->src[2].type == BRW_REGISTER_TYPE_DF);
> +            assert(inst->group == 0 || inst->group == 4);
> +            brw_inst_set_group(p->devinfo, p->current, inst->group);

I think this should be unconditional (e.g. if somebody tries to set
group == 4 on an instruction with exec_size == 8 it would be nice to get
an assertion failure instead of the group value being silently
ignored).  How about you replace the 'brw_set_default_group(p, 0)' line
above with:

|
|       assert(inst->group % inst->exec_size == 0);
|       assert(inst->group % 8 == 0 ||
|              inst->dst.type == BRW_REGISTER_TYPE_DF ||
|              inst->src[0].type == BRW_REGISTER_TYPE_DF ||
|              inst->src[1].type == BRW_REGISTER_TYPE_DF ||
|              inst->src[2].type == BRW_REGISTER_TYPE_DF);
|       brw_set_default_group(p, inst->group);

and then drop this hunk?

>        }
>  
>        switch (inst->opcode) {
> -- 
> 2.7.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160808/6141ac82/attachment.sig>