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