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 < za...@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 < za...@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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev