[Mesa-dev] [RFC 3/5] i965/fs: Plumb separate surfaces and samplers through from NIR

Jason Ekstrand jason at jlekstrand.net
Wed Nov 25 18:57:51 PST 2015


On Wed, Nov 25, 2015 at 4:13 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Tuesday, November 03, 2015 01:26:02 PM Jason Ekstrand wrote:
>> ---
>>  src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp |  2 +-
>>  src/mesa/drivers/dri/i965/brw_fs.cpp            | 47 +++++++++++++++----------
>>  src/mesa/drivers/dri/i965/brw_fs.h              |  4 ++-
>>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp  |  2 +-
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp        | 25 +++++++++----
>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp    | 16 +++++----
>>  6 files changed, 60 insertions(+), 36 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
>> index 5308d17..7fa4ce8 100644
>> --- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
>> @@ -85,7 +85,7 @@ brw_blorp_eu_emitter::emit_texture_lookup(const struct brw_reg &dst,
>>                                            unsigned msg_length)
>>  {
>>     fs_inst *inst = new (mem_ctx) fs_inst(op, 16, dst, brw_message_reg(base_mrf),
>> -                                         fs_reg(0u));
>> +                                         fs_reg(0u), fs_reg(0u));
>>
>>     inst->base_mrf = base_mrf;
>>     inst->mlen = msg_length;
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 2d0acb9..92fa02f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -723,15 +723,15 @@ fs_inst::components_read(unsigned i) const
>>     case SHADER_OPCODE_LOD_LOGICAL:
>>     case SHADER_OPCODE_TG4_LOGICAL:
>>     case SHADER_OPCODE_TG4_OFFSET_LOGICAL:
>> -      assert(src[8].file == IMM && src[9].file == IMM);
>> +      assert(src[9].file == IMM && src[10].file == IMM);
>>        /* Texture coordinates. */
>>        if (i == 0)
>> -         return src[8].fixed_hw_reg.dw1.ud;
>> +         return src[9].fixed_hw_reg.dw1.ud;
>
> Okay, this part is maddening...you decided to stick the source in the
> middle, and now every value on the planet has to change.  It's
> definitely the right place to put it, though.

Yes, it was maddening... Writing that patch was a real pain.

> I'd like to see a patch before this which introduces an enum for logical
> texture sources, similar to enum fb_write_logical_srcs.  Once everything
> is converted to enums, we can easily just stick a new enum value in the
> middle without having to edit so much code.

Just an enum with something like

FS_TEX_SRC_COORD
FS_TEX_SRC_DIM

etc.?

> You also didn't update the opcode documentation for the new sources;
> adding an enum would solve that problem too.
>
>>        /* Texture derivatives. */
>>        else if ((i == 2 || i == 3) && opcode == SHADER_OPCODE_TXD_LOGICAL)
>> -         return src[9].fixed_hw_reg.dw1.ud;
>> +         return src[10].fixed_hw_reg.dw1.ud;
>>        /* Texture offset. */
>> -      else if (i == 7)
>> +      else if (i == 8)
>>           return 2;
>>        else
>>           return 1;
>> @@ -3505,6 +3505,7 @@ lower_sampler_logical_send_gen4(const fs_builder &bld, fs_inst *inst, opcode op,
>>                                  const fs_reg &coordinate,
>>                                  const fs_reg &shadow_c,
>>                                  const fs_reg &lod, const fs_reg &lod2,
>> +                                const fs_reg &surface,
>>                                  const fs_reg &sampler,
>>                                  unsigned coord_components,
>>                                  unsigned grad_components)
>> @@ -3597,8 +3598,9 @@ lower_sampler_logical_send_gen4(const fs_builder &bld, fs_inst *inst, opcode op,
>>
>>     inst->opcode = op;
>>     inst->src[0] = reg_undef;
>> -   inst->src[1] = sampler;
>> -   inst->resize_sources(2);
>> +   inst->src[1] = surface;
>> +   inst->src[2] = sampler;
>> +   inst->resize_sources(3);
>>     inst->base_mrf = msg_begin.reg;
>>     inst->mlen = msg_end.reg - msg_begin.reg;
>>     inst->header_size = 1;
>> @@ -3610,6 +3612,7 @@ lower_sampler_logical_send_gen5(const fs_builder &bld, fs_inst *inst, opcode op,
>>                                  const fs_reg &shadow_c,
>>                                  fs_reg lod, fs_reg lod2,
>>                                  const fs_reg &sample_index,
>> +                                const fs_reg &surface,
>>                                  const fs_reg &sampler,
>>                                  const fs_reg &offset_value,
>>                                  unsigned coord_components,
>> @@ -3692,8 +3695,9 @@ lower_sampler_logical_send_gen5(const fs_builder &bld, fs_inst *inst, opcode op,
>>
>>     inst->opcode = op;
>>     inst->src[0] = reg_undef;
>> -   inst->src[1] = sampler;
>> -   inst->resize_sources(2);
>> +   inst->src[1] = surface;
>> +   inst->src[2] = sampler;
>> +   inst->resize_sources(3);
>>     inst->base_mrf = message.reg;
>>     inst->mlen = msg_end.reg - message.reg;
>>     inst->header_size = header_size;
>> @@ -3717,7 +3721,9 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,
>>                                  const fs_reg &shadow_c,
>>                                  fs_reg lod, fs_reg lod2,
>>                                  const fs_reg &sample_index,
>> -                                const fs_reg &mcs, const fs_reg &sampler,
>> +                                const fs_reg &mcs,
>> +                                const fs_reg &surface,
>> +                                const fs_reg &sampler,
>>                                  fs_reg offset_value,
>>                                  unsigned coord_components,
>>                                  unsigned grad_components)
>> @@ -3906,8 +3912,9 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,
>>     /* Generate the SEND. */
>>     inst->opcode = op;
>>     inst->src[0] = src_payload;
>> -   inst->src[1] = sampler;
>> -   inst->resize_sources(2);
>> +   inst->src[1] = surface;
>> +   inst->src[2] = sampler;
>> +   inst->resize_sources(3);
>>     inst->base_mrf = -1;
>>     inst->mlen = mlen;
>>     inst->header_size = header_size;
>> @@ -3926,25 +3933,27 @@ lower_sampler_logical_send(const fs_builder &bld, fs_inst *inst, opcode op)
>>     const fs_reg &lod2 = inst->src[3];
>>     const fs_reg &sample_index = inst->src[4];
>>     const fs_reg &mcs = inst->src[5];
>> -   const fs_reg &sampler = inst->src[6];
>> -   const fs_reg &offset_value = inst->src[7];
>> -   assert(inst->src[8].file == IMM && inst->src[9].file == IMM);
>> -   const unsigned coord_components = inst->src[8].fixed_hw_reg.dw1.ud;
>> -   const unsigned grad_components = inst->src[9].fixed_hw_reg.dw1.ud;
>> +   const fs_reg &surface = inst->src[6];
>> +   const fs_reg &sampler = inst->src[7];
>> +   const fs_reg &offset_value = inst->src[8];
>> +   assert(inst->src[9].file == IMM && inst->src[10].file == IMM);
>> +   const unsigned coord_components = inst->src[9].fixed_hw_reg.dw1.ud;
>> +   const unsigned grad_components = inst->src[10].fixed_hw_reg.dw1.ud;
>>
>>     if (devinfo->gen >= 7) {
>>        lower_sampler_logical_send_gen7(bld, inst, op, coordinate,
>>                                        shadow_c, lod, lod2, sample_index,
>> -                                      mcs, sampler, offset_value,
>> +                                      mcs, surface, sampler, offset_value,
>>                                        coord_components, grad_components);
>>     } else if (devinfo->gen >= 5) {
>>        lower_sampler_logical_send_gen5(bld, inst, op, coordinate,
>>                                        shadow_c, lod, lod2, sample_index,
>> -                                      sampler, offset_value,
>> +                                      surface, sampler, offset_value,
>>                                        coord_components, grad_components);
>>     } else {
>>        lower_sampler_logical_send_gen4(bld, inst, op, coordinate,
>> -                                      shadow_c, lod, lod2, sampler,
>> +                                      shadow_c, lod, lod2,
>> +                                      surface, sampler,
>>                                        coord_components, grad_components);
>>     }
>>  }
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
>> index b06a069..ec38693 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -114,7 +114,7 @@ public:
>>     void setup_uniform_clipplane_values(gl_clip_plane *clip_planes);
>>     void compute_clip_distance(gl_clip_plane *clip_planes);
>>
>> -   uint32_t gather_channel(int orig_chan, uint32_t sampler);
>> +   uint32_t gather_channel(int orig_chan, uint32_t surface, uint32_t sampler);
>>     void swizzle_result(ir_texture_opcode op, int dest_components,
>>                         fs_reg orig_val, uint32_t sampler);
>>
>> @@ -225,6 +225,8 @@ public:
>>                       int gather_component,
>>                       bool is_cube_array,
>>                       bool is_rect,
>> +                     uint32_t surface,
>> +                     fs_reg surface_reg,
>>                       uint32_t sampler,
>>                       fs_reg sampler_reg);
>>     fs_reg emit_mcs_fetch(const fs_reg &coordinate, unsigned components,
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> index 14a8f29..d1a4dc4 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> @@ -2061,7 +2061,7 @@ fs_generator::generate_code(const cfg_t *cfg, int dispatch_width)
>>        case SHADER_OPCODE_TG4:
>>        case SHADER_OPCODE_TG4_OFFSET:
>>        case SHADER_OPCODE_SAMPLEINFO:
>> -      generate_tex(inst, dst, src[0], src[1], src[1]);
>> +      generate_tex(inst, dst, src[0], src[1], src[2]);
>>        break;
>>        case FS_OPCODE_DDX_COARSE:
>>        case FS_OPCODE_DDX_FINE:
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index 81cabaf..db859c3 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -1984,7 +1984,9 @@ fs_visitor::nir_emit_ssbo_atomic(const fs_builder &bld,
>>  void
>>  fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
>>  {
>> +   unsigned texture = instr->texture_index;
>>     unsigned sampler = instr->sampler_index;
>> +   fs_reg texture_reg(texture);
>>     fs_reg sampler_reg(sampler);
>>
>>     int gather_component = instr->component;
>> @@ -2052,9 +2054,9 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
>>        case nir_tex_src_projector:
>>           unreachable("should be lowered");
>>
>> -      case nir_tex_src_sampler_offset: {
>> -         /* Figure out the highest possible sampler index and mark it as used */
>> -         uint32_t max_used = sampler + instr->texture_array_size - 1;
>> +      case nir_tex_src_texture_offset: {
>> +         /* Figure out the highest possible texture index and mark it as used */
>> +         uint32_t max_used = texture + instr->texture_array_size - 1;
>>           if (instr->op == nir_texop_tg4 && devinfo->gen < 8) {
>>              max_used += stage_prog_data->binding_table.gather_texture_start;
>>           } else {
>> @@ -2063,6 +2065,14 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
>>           brw_mark_surface_used(prog_data, max_used);
>>
>>           /* Emit code to evaluate the actual indexing expression */
>> +         texture_reg = vgrf(glsl_type::uint_type);
>
> The newer way to say this is:
>
>    texture_reg = bld.vgrf(BRW_REGISTER_TYPE_UD);


More information about the mesa-dev mailing list