[Mesa-dev] [PATCH] tgsi: document and fixup IF and BREAKC instrunctions

Thu Apr 11 13:53:26 PDT 2013

Actually our svga driver seems to think that the IF opcode works like
that. Since it will translate it into a SVGA3DOP_IFC (which corresponds
to shader model 2 if_comp which compares to sources and specifies a
comparison function - my guess is this is what the unused
TGSI_OPCODE_IFC was meant for, but clearly it's failure at that since
not even the number of arguments would be right).
Still, nothing emitting the OPCODE_IF actually thinks the condition is a
float which should be tested for zero, so that was sketchy as well. I
guess this is done because we don't have the "true" 1-bit bools of
shader models 2_x and 3, and the "reasonable" IF instruction isn't
available until shader model 4.

Roland

Am 11.04.2013 22:17, schrieb Jose Fonseca:
> Ah.. This indeed rings a bell. I don't recall the details but I'm pretty sure I was against dual semantics. And the fact that this problem is again showing its ugly head is the proof of it.
> 
> We really should have two IF opcodes. And the state tracker should choose which one it wants.
> 
> Jose
> 
> ----- Original Message -----
>> What about drivers without integer support? The IF instruction should have 2
>> definitions: one for the drivers which support PIPE_SHADER_CAP_INTEGERS, and
>> the other one for those which don't. Obviously, there is no way to change
>> the behavior of the IF opcode if you only have floats.
>>
>> Marek
>>
>>
>> On Thu, Apr 11, 2013 at 5:20 AM, Zack Rusin < zackr at vmware.com > wrote:
>>
>>
>> As implemented in tgsi_exec they both test src operands on the bit
>> level and don't do floating point comperisons as some thought.
>> This documents them as such to avoid future confusion and fixes
>> their implementation in llvmpipe.
>>
>> Could gallium driver developers make sure that they're handling
>> those instrunctions correctly? From my quick glance it seems
>> to be ok, but I don't know those codebases at all.
>>
>> Signed-off-by: Zack Rusin < zackr at vmware.com >
>> ---
>> src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 10 ++++------
>> src/gallium/auxiliary/tgsi/tgsi_info.c | 2 ++
>> src/gallium/docs/source/tgsi.rst | 16 ++++++++++------
>> 3 files changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
>> b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
>> index 853de09..8a29635 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
>> @@ -2370,12 +2370,9 @@ breakc_emit(
>> struct lp_build_emit_data * emit_data)
>> {
>> struct lp_build_tgsi_soa_context * bld = lp_soa_context(bld_base);
>> - LLVMBuilderRef builder = bld_base->base.gallivm->builder;
>> struct lp_build_context *uint_bld = &bld_base->uint_bld;
>> - LLVMValueRef unsigned_cond =
>> - LLVMBuildBitCast(builder, emit_data->args[0], uint_bld->vec_type, "");
>> LLVMValueRef cond = lp_build_cmp(uint_bld, PIPE_FUNC_NOTEQUAL,
>> - unsigned_cond,
>> + emit_data->args[0],
>> uint_bld->zero);
>>
>> lp_exec_break_condition(&bld->exec_mask, cond);
>> @@ -2389,9 +2386,10 @@ if_emit(
>> {
>> LLVMValueRef tmp;
>> struct lp_build_tgsi_soa_context * bld = lp_soa_context(bld_base);
>> + struct lp_build_context *uint_bld = &bld_base->uint_bld;
>>
>> - tmp = lp_build_cmp(&bld_base->base, PIPE_FUNC_NOTEQUAL,
>> - emit_data->args[0], bld->bld_base.base.zero);
>> + tmp = lp_build_cmp(uint_bld, PIPE_FUNC_NOTEQUAL,
>> + emit_data->args[0], uint_bld->zero);
>> lp_exec_mask_cond_push(&bld->exec_mask, tmp);
>> }
>>
>> diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c
>> b/src/gallium/auxiliary/tgsi/tgsi_info.c
>> index 1fadfec..f488351 100644
>> --- a/src/gallium/auxiliary/tgsi/tgsi_info.c
>> +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c
>> @@ -297,6 +297,8 @@ tgsi_opcode_infer_src_type( uint opcode )
>> case TGSI_OPCODE_TXF:
>> case TGSI_OPCODE_SAMPLE_I:
>> case TGSI_OPCODE_SAMPLE_I_MS:
>> + case TGSI_OPCODE_IF:
>> + case TGSI_OPCODE_BREAKC:
>> return TGSI_TYPE_UNSIGNED;
>> case TGSI_OPCODE_MOD:
>> case TGSI_OPCODE_I2F:
>> diff --git a/src/gallium/docs/source/tgsi.rst
>> b/src/gallium/docs/source/tgsi.rst
>> index 28308cb..e6d97d6 100644
>> --- a/src/gallium/docs/source/tgsi.rst
>> +++ b/src/gallium/docs/source/tgsi.rst
>> @@ -863,10 +863,19 @@ This instruction replicates its result.
>>
>> TBD
>>
>> +
>> +.. opcode:: BREAKC - Break Conditional
>> +
>> + Conditionally moves the point of execution to the instruction after the
>> + next endloop or endswitch. The instruction must appear within a
>> loop/endloop
>> + or switch/endswitch. The 32-bit register supplied by src0 is tested at a
>> + bit level (treat it as unsigned).
>> +
>>
>> .. opcode:: IF - If
>>
>> - TBD
>> + Branch based on logical OR result. The 32-bit register supplied by src0 is
>> + tested at a bit level. If any bit is nonzero, it will evaluate to true.
>>
>>
>> .. opcode:: ELSE - Else
>> @@ -1202,11 +1211,6 @@ XXX wait what
>>
>> TBD
>>
>> -
>> -.. opcode:: BREAKC - Break Conditional
>> -
>> - TBD
>> -
>> .. _doubleopcodes:
>>
>> Double ISA
>> --
>> 1.7.10.4
>>
>> _______________________________________________
>> 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
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 