[Mesa-dev] [PATCH 8/8] i965: Enable EXT_shader_samples_identical
Jason Ekstrand
jason at jlekstrand.net
Wed Nov 18 17:51:37 PST 2015
On Wed, Nov 18, 2015 at 5:31 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 11/18/2015 05:02 PM, Jason Ekstrand wrote:
>> 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.
>
> Derp. Yeah, that's true.
>
>>>> + } 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.
>
> Does that only apply to clear colors that are compatible with
> fast-clear? In my simple test, it appears that the cleared area returns
> all zeros.
Yes, that's means fast-clear color.
>> 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.
>
> Oof. I think it has, but I don't think I have a 16x-compatible
> platform. I guess we could always return false for now if the sampler
> is 16x.
That would work for now. That said, it wouldn't be that hard to loop
over the result and ensure all values are zero. However, that
requires that we know how many samples we have.
>> --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
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list