[Mesa-dev] [PATCH 14/15] i965/vs: Always store pull constant offsets in GRFs on Gen8.

[Paul Berry]

On 1 December 2013 00:44, Kenneth Graunke <kenneth at whitecape.org> wrote:

> On 11/30/2013 10:40 AM, Paul Berry wrote:
> > On 12 November 2013 17:51, Kenneth Graunke <kenneth at whitecape.org
> > <mailto:kenneth at whitecape.org>> wrote:
> >
> >     We need to SEND from a GRF, and we can only obtain those prior to
> >     register allocation.
> >
> >     This allows us to do pull constant loads without the MRF hack.
> >
> >     Signed-off-by: Kenneth Graunke <kenneth at whitecape.org
> >     <mailto:kenneth at whitecape.org>>
> >     ---
> >      src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 13 ++++++++++++-
> >      1 file changed, 12 insertions(+), 1 deletion(-)
> >
> >     diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> >     b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> >     index a036e2d..5f0d0b4 100644
> >     --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> >     +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> >     @@ -1582,7 +1582,13 @@ vec4_visitor::visit(ir_expression *ir)
> >            src_reg surf_index =
> >               src_reg(prog_data->base.binding_table.ubo_start +
> >     uniform_block->value.u[0]);
> >            if (const_offset_ir) {
> >     -         offset = src_reg(const_offset / 16);
> >     +         if (brw->gen >= 8) {
> >     +            /* Put the offset in a GRF; we can't SEND from
> >     immediates. */
> >     +            offset = src_reg(this, glsl_type::int_type);
> >     +            emit(MOV(dst_reg(offset), src_reg(const_offset / 16)));
> >     +         } else {
> >     +            offset = src_reg(const_offset / 16);
> >     +         }
> >            } else {
> >               offset = src_reg(this, glsl_type::uint_type);
> >               emit(SHR(dst_reg(offset), op[1], src_reg(4)));
> >     @@ -2983,6 +2989,11 @@
> >     vec4_visitor::get_pull_constant_offset(vec4_instruction *inst,
> >            }
> >
> >            return index;
> >     +   } else if (brw->gen >= 8) {
> >     +      /* Put the offset in a GRF; we can't SEND from immediates. */
> >     +      src_reg offset = src_reg(this, glsl_type::int_type);
> >     +      emit_before(inst, MOV(dst_reg(offset), src_reg(reg_offset)));
> >     +      return offset;
> >         } else {
> >            int message_header_scale = brw->gen < 6 ? 16 : 1;
> >            return src_reg(reg_offset * message_header_scale);
> >     --
> >     1.8.3.2
> >
> >
> > Looking at the docs, it appears to me that SENDs from immediates have
> > never been allowed.  Is it possible that they have been working just by
> > luck?  Maybe we should make this change apply regardless of brw->gen.
>
> Not by luck...by MRFs.
>
> (Gen4-7) vec4_generator handles the VS_OPCODE_PULL_CONSTANT_LOAD opcode
> by emitting MOVs to a MRF (or fake MRF on Gen7).  It's treated as a
> OWORD_DUAL_BLOCK_READ, and is never headerless.
>
> On Gen8, I chose to make both VS_OPCODE_PULL_CONSTANT_LOAD and
> VS_OPCODE_PULL_CONSTANT_LOAD_GEN7 use the same method (the ld sampler
> message), which doesn't require a header, and thus is easy to send from
> a GRF.  We just can't use immediates.
>
> Looking at the situation now...it seems pretty stupid that we use
> VS_OPCODE_PULL_CONSTANT_LOAD on Gen7+ at all.  Data port reads are
> ridiculously slow on Ivybridge, and still pretty bad on Haswell.  And we
> use them for UBOs.  We should probably convert those, which would
> require doing this code on Gen7 as well.
>

Ok, I see.  Can you update the commit message and the code comments to
clarify this?  As is, when you say "we can't SEND from immediates", it
sounds like you're saying that this is a new hardware restriction for Gen8,
and that's why I was confused.

With that changed, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131201/43145f13/attachment.html>