[Mesa-dev] [PATCH 8/8] i965: Enable EXT_shader_samples_identical

[Jason Ekstrand]

On Wed, Nov 18, 2015 at 4:06 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Wednesday, November 18, 2015 03:46:54 PM Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp       |  1 +
>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp   | 16 ++++++++++++++++
>>  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp     |  1 +
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 11 +++++++++++
>>  src/mesa/drivers/dri/i965/intel_extensions.c   |  1 +
>>  5 files changed, 30 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index 1f71f66..4af1234 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -2550,6 +2550,7 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)
>>           switch (instr->op) {
>>           case nir_texop_txf:
>>           case nir_texop_txf_ms:
>> +         case nir_texop_samples_identical:
>>              coordinate = retype(src, BRW_REGISTER_TYPE_D);
>>              break;
>>           default:
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> index a7bd9ce..6688f6a 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> @@ -259,6 +259,22 @@ fs_visitor::emit_texture(ir_texture_opcode op,
>>        lod = fs_reg(0u);
>>     }
>>
>> +   if (op == ir_samples_identical) {
>> +      fs_reg dst = vgrf(glsl_type::get_instance(dest_type->base_type, 1, 1));
>> +
>> +      if (mcs.file == BRW_IMMEDIATE_VALUE) {
>> +         fs_reg tmp = vgrf(glsl_type::uint_type);
>> +
>> +         bld.MOV(tmp, mcs);
>> +         bld.CMP(dst, tmp, src_reg(0u), BRW_CONDITIONAL_EQ);
>
> Seems a little strange to emit assembly to do the comparison when
> you've already determined that the value is a compile time constant.
>
> Why not just:
>
>    bld.MOV(dst, fs_reg(mcs.ud == 0u ? ~0u : 0u));

Actually, getting an immediate here means we don't have an MCS and we
have no idea of the samples are identical, so we should return false
always.

>> +      } else {
>> +         bld.CMP(dst, mcs, src_reg(0u), BRW_CONDITIONAL_EQ);

We should also consider handling the clear color case.  In this case,
we'll get 0xff for 2x and 0xffffffff for 4x or 8x.  Do we know the
number of samples in the shader?  We should be able to get that from
the sampler or something but then we would have to pass that through
the key and that would get gross.

One other thought, 16x MSAA will break all this because it gives you a
ivec4 value from the MCS (if I remember correctly).  Not sure if we've
landed 16x MSAA yet though.
--Jason

>> +      }
>> +
>> +      this->result = dst;
>> +      return;
>> +   }
>> +
>>     if (coordinate.file != BAD_FILE) {
>>        /* FINISHME: Texture coordinate rescaling doesn't work with non-constant
>>         * samplers.  This should only be a problem with GL_CLAMP on Gen7.
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> index 3c2674d..41c3c10 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -1615,6 +1615,7 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr)
>>           switch (instr->op) {
>>           case nir_texop_txf:
>>           case nir_texop_txf_ms:
>> +         case nir_texop_samples_identical:
>>              coordinate = get_nir_src(instr->src[i].src, BRW_REGISTER_TYPE_D,
>>                                       src_size);
>>              coord_type = glsl_type::ivec(src_size);
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> index fda3d7c..2190a86 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> @@ -909,6 +909,17 @@ vec4_visitor::emit_texture(ir_texture_opcode op,
>>        unreachable("TXB is not valid for vertex shaders.");
>>     case ir_lod:
>>        unreachable("LOD is not valid for vertex shaders.");
>> +   case ir_samples_identical: {
>> +      if (mcs.file == BRW_IMMEDIATE_VALUE) {
>> +         const src_reg temp = src_reg(this, glsl_type::uint_type);
>> +
>> +         emit(MOV(dst_reg(temp), mcs));
>> +         emit(CMP(dest, temp, src_reg(0u), BRW_CONDITIONAL_EQ));
>
> Ditto.
>
>    bld.MOV(dst, src_reg(mcs.ud == 0u ? ~0u : 0u));
>
>> +      } else {
>> +         emit(CMP(dest, mcs, src_reg(0u), BRW_CONDITIONAL_EQ));
>> +      }
>> +      return;
>> +   }
>>     default:
>>        unreachable("Unrecognized tex op");
>>     }
>> diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c
>> index 386b63c..2e2459c 100644
>> --- a/src/mesa/drivers/dri/i965/intel_extensions.c
>> +++ b/src/mesa/drivers/dri/i965/intel_extensions.c
>> @@ -333,6 +333,7 @@ intelInitExtensions(struct gl_context *ctx)
>>        ctx->Extensions.ARB_texture_compression_bptc = true;
>>        ctx->Extensions.ARB_texture_view = true;
>>        ctx->Extensions.ARB_shader_storage_buffer_object = true;
>> +      ctx->Extensions.EXT_shader_samples_identical = true;
>>
>>        if (can_do_pipelined_register_writes(brw)) {
>>           ctx->Extensions.ARB_draw_indirect = true;
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>