[Mesa-dev] [PATCH] i965/vec4: Fix the source register for indexed samplers
neil at linux.intel.com
Thu Jun 4 05:59:43 PDT 2015
Matt Turner <mattst88 at gmail.com> writes:
> 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.
Yes sorry, I probably didn't explain it very well. Your explanation is a
> 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.
>> + 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?
Yes, that probably makes sense. I was a bit reluctant to do this because
there are other places that are calling brw_send_indirect_message and I
wasn't entirely sure what impact the change would have on them. To be
honest I don't think I understand the base_mrf thing very well. My
understanding is that after Sandybridge the MRF registers don't really
exist and you can just send from any GRF register. However even on
Sandybridge it looks like you don't necessarily have to use the base_mrf
thing and you can just allocate a register using the normal mechanism
but mark it as being in the MRF. Is that correct? Is base_mrf
effectively a historical thing that was used before we could allocate
registers directly in the MRF? If so, yes, maybe it would be nice to do
a proper cleanup and get rid of it entirely at some point and just use
the normal register allocation mechanism to get MRF registers.
I'll have another look at moving it into brw_send_indirect_message.
Thanks for the review.
More information about the mesa-dev