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

[Zack Rusin]

----- Original Message -----
> Am 11.04.2013 05:20, schrieb Zack Rusin:
> > 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.
> Maybe something similar to the "If any bit is nonzero, ..." part could
> be added to the breakc comment as well?
> Also I'm not convinced that "Branch based on logical OR result" makes
> things really clearer.

Good point. How about:

.. 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). If any bit is nonzero, it will evaluate
  to true.


.. opcode:: IF - If

  Branch if the condition evaluates to true. The 32-bit register supplied by
  src0 is tested at a bit level. If any bit is nonzero, it will evaluate
  to true.

?