On Thu, May 28, 2015 at 11:35 AM, Neil Roberts <n...@linux.intel.com> wrote: > Previously when setting up the sample instruction for an indirect > sampler the vec4 backend was directly passing the pseudo opcode's > src0. However this isn't actually set to a valid register because > instead the MRF registers are used as the source so it would end up > passing null as src0. > > This patch makes it call gen6_resolve_implied_move before setting up > the indirect message. This is similar to what is done for constant > sampler numbers in brw_SAMPLE.
I don't know why I was confused by this patch -- after arriving at the same conclusion independently I see that all of the analysis I needed was right there. To sum up, vec4_visitor::visit(ir_texture *) doesn't set the texture operation's src0 -- it's left as BAD_FILE, which when translated into a brw_reg gives the null register. In brw_SAMPLE, gen6_resolve_implied_move() inserts a MOV from the inst->base_mrf and sets the src0 appropriately. The indirect sampler case does not have a call to gen6_resolve_implied_move(). How convoluted. The fs backend avoids this because the platforms that support dynamic indexing of samplers (IVB+) have been converted to not use the fake-MRF hack, and instead send from proper GRFs. Ideally, we'd convert the vec4 backend to use load_payload and send-from-GRF. Even with that though, I think it would be nice to use a proper MRF source but I'm not sure how much work is involved. I know we have some src[i].file != MRF assertions sprinkled around that would have to change at least. > The Piglit tests for sampler array indexing didn't pick this up > because they were using a texture with a solid colour so it didn't > matter what texture coordinates were actually used. I've posted a > patch for Piglit to make the tests more thorough here: > > http://lists.freedesktop.org/archives/piglit/2015-May/016127.html > > With that patch the tests for gs and vs are currently failing on > Ivybridge, but this patch fixes them. There are no other changes to a > Piglit run on Ivybridge. Thanks for doing that. I'll take a look. > On Skylake the gs tests were failing even without the Piglit patch > because Skylake needs the source registers to work correctly in order > to send a message header to select SIMD4x2 mode. > --- > src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > index ef77b8d..20d096c 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp > @@ -422,6 +422,9 @@ vec4_generator::generate_tex(vec4_instruction *inst, > > brw_pop_insn_state(p); > > + if (inst->base_mrf != -1) > + gen6_resolve_implied_move(p, &src, inst->base_mrf); I think it maybe makes more sense to add this to brw_send_indirect_message(), to make it more symmetrical with brw_SAMPLE. What do you think? > + > /* dst = send(offset, a0.0 | <descriptor>) */ > brw_inst *insn = brw_send_indirect_message( > p, BRW_SFID_SAMPLER, dst, src, addr); > -- > 1.9.3 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
