<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 1, 2016 at 3:58 PM, Francisco Jerez <span dir="ltr"><<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes:<br>
<br>
> On Wed, Jun 1, 2016 at 3:33 PM, Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>><br>
> wrote:<br>
><br>
>> Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes:<br>
>><br>
>> > This fixes 64 Vulkan CTS tests per gen<br>
>> ><br>
>> > Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=96299" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=96299</a><br>
>> > Cc: "12.0" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>><br>
>> > Cc: Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>><br>
>> > Cc: Mark Janes <<a href="mailto:mark.a.janes@intel.com">mark.a.janes@intel.com</a>><br>
>> > ---<br>
>> > src/mesa/drivers/dri/i965/brw_fs.cpp | 8 ++++++++<br>
>> > 1 file changed, 8 insertions(+)<br>
>> ><br>
>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
>> > index 00d937e..20bb900 100644<br>
>> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
>> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
>> > @@ -4448,6 +4448,14 @@ lower_varying_pull_constant_logical_send(const<br>
>> fs_builder &bld, fs_inst *inst)<br>
>> > const brw_device_info *devinfo = bld.shader->devinfo;<br>
>> ><br>
>> > if (devinfo->gen >= 7) {<br>
>> > + /* We are switching the instruction from an ALU-like instruction<br>
>> to a<br>
>> > + * send-from-grf instruction. Since sends can't handle strides or<br>
>> > + * source modifiers, we have to make a copy of the offset source.<br>
>> > + */<br>
>> > + fs_reg tmp = bld.vgrf(inst->src[1].type);<br>
>><br>
>> I suggest you use a fixed UD type for the temporary (since that is what<br>
>> the varying pull constant load instruction requires and using any other<br>
>> type will cause the send-message to silently reinterpret the offset as<br>
>> something else than what it is. E.g. think non-32bit integers or<br>
>> integer vectors, a type-converting move is what you want below in such<br>
>> cases). With that change this patch is:<br>
>><br>
><br>
> Hrm... How would we ever get anything in there other than a UD? If<br>
> something else, say a float, got copy-propagated in here, that seems to be<br>
> a problem with the optimizer not lowering. I'm fine making the change if<br>
> you'd like.<br>
><br>
</div></div>If you don't expect anything else than a UD as source, how about<br>
'assert(...type == BRW_REGISTER_TYPE_UD)'? I think it should be okay<br>
either way, I suggested using a fixed type above instead because that<br>
way you'd save an artificial restriction on the source register type of<br>
this virtual opcode, making the assertion redundant.<br></blockquote><div><br></div><div>Fair enough. Certainly, an implicit format conversion is a better failure mode than a GPU hang or something similarly stupid.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">
> Reviewed-by: Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>><br>
>><br>
><br>
> Thanks!<br>
><br>
><br>
>><br>
>> > + bld.MOV(tmp, inst->src[1]);<br>
>> > + inst->src[1] = tmp;<br>
>> > +<br>
>> > inst->opcode = FS_OPCODE_VARYING_PULL_CONSTANT_LOAD_GEN7;<br>
>> ><br>
>> > } else {<br>
>> > --<br>
>> > 2.5.0.400.gff86faf<br>
>> ><br>
>> > _______________________________________________<br>
>> > mesa-stable mailing list<br>
>> > <a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a><br>
>> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-stable" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-stable</a><br>
>><br>
</div></div></blockquote></div><br></div></div>