On Tue, Mar 7, 2017 at 5:11 AM, Samuel Iglesias Gonsálvez
<sigles...@igalia.com> wrote:
>
>
> On 04/03/17 01:44, Francisco Jerez wrote:
>> Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes:
>>
>>> From: Matt Turner <matts...@gmail.com>
>>>
>>> In commit c35fa7a, we changed the "width" of DF source registers to 2,
>>> which is conceptually fine. Unfortunately a VertStride of 2 is not
>>> allowed by align16 instructions on IVB/BYT, and the regular VertStride
>>> of 4 works fine in any case.
>>>
>>> See
>>> generated_tests/spec/arb_gpu_shader_fp64/execution/built-in-functions/vs-round-double.shader_test
>>> for example:
>>>
>>> cmp.ge.f0(8) g18<1>DF g1<0>.xyxyDF -g8<2>DF { align16
>>> 1Q };
>>> ERROR: In Align16 mode, only VertStride of 0 or 4 is allowed
>>> cmp.ge.f0(8) g19<1>DF g1<0>.xyxyDF -g9<2>DF { align16
>>> 2N };
>>> ERROR: In Align16 mode, only VertStride of 0 or 4 is allowed
>>>
>>> v2:
>>> - Add spec quote (Curro).
>>> - Change the condition to only BRW_VERTICAL_STRIDE_2 (Curro)
>>>
>>> Reviewed-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
>>> ---
>>> src/mesa/drivers/dri/i965/brw_eu_emit.c | 44
>>> +++++++++++++++++++++++++--------
>>> 1 file changed, 34 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>>> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>>> index 03aaa760163..d221405db4d 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
>>> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
>>> @@ -512,13 +512,25 @@ brw_set_src0(struct brw_codegen *p, brw_inst *inst,
>>> struct brw_reg reg)
>>> brw_inst_set_src0_da16_swiz_w(devinfo, inst,
>>> BRW_GET_SWZ(reg.swizzle, BRW_CHANNEL_W));
>>>
>>> - /* This is an oddity of the fact we're using the same
>>> - * descriptions for registers in align_16 as align_1:
>>> - */
>>> - if (reg.vstride == BRW_VERTICAL_STRIDE_8)
>>> + if (reg.vstride == BRW_VERTICAL_STRIDE_8) {
>>> + /* This is an oddity of the fact we're using the same
>>> + * descriptions for registers in align_16 as align_1:
>>> + */
>>> + brw_inst_set_src0_vstride(devinfo, inst,
>>> BRW_VERTICAL_STRIDE_4);
>>> + } else if (devinfo->gen == 7 && !devinfo->is_haswell &&
>>> + reg.type == BRW_REGISTER_TYPE_DF &&
>>> + reg.vstride == BRW_VERTICAL_STRIDE_2) {
>>> + /* From HSW PRM:
>>
>> This workaround is IVB-specific so the HSW PRM quotation doesn't seem
>> particularly relevant. With that fixed here and below patch is:
>>
>
> The problem is that this quote is not present in IVB PRM, hence the
> sentence: "Presumably the DevSNB behavior applies to IVB as well."
>
> Looks like it is yet another omission in the PRMs. Adding Matt on Cc
> just in case he wants to add something.
Right. As I said before [1], I cannot find anything in the IVB PRM. I
can only determine that VertStride=2 is not legal on IVB by inference,
since the internal documentation says SNB doesn't support
VertStride=2, and *HSW* does support VertStride=2.
I think Curro would be happy if the comment just said exactly what my
email [1] said.
[1] https://lists.freedesktop.org/archives/mesa-dev/2017-January/141744.html
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
