I think that we should probably allow signed everywhere. The operation and result may be unsigned, but the arguments may need to be negated to become unsigned.
Imagine: int a = -1; unsigned b = 10 / (unsigned)-a; That is MOV TEMP[0], INT IMM{-1} UDIV TEMP[1], UINT IMM{10}, -TEMP[0] Jose ----- Original Message ----- > From: Roland Scheidegger <srol...@vmware.com> > > Need to take the type into account. Also, if we want to allow > mov's with modifiers we need to pick a type (assume float). > > v2: don't allow all modifiers on all type, in particular don't allow > absolute on non-float types and don't allow negate on unsigned. > Also treat UADD as signed (despite the name) since it is used > for handling both signed and unsigned integer arguments and otherwise > modifiers don't work. > Also add tgsi docs clarifying this. > --- > src/gallium/auxiliary/gallivm/lp_bld_tgsi.c | 36 > +++++++++++++++++++++++++-- > src/gallium/auxiliary/tgsi/tgsi_exec.c | 2 +- > src/gallium/auxiliary/tgsi/tgsi_info.c | 6 +++-- > src/gallium/docs/source/tgsi.rst | 15 +++++++++++ > 4 files changed, 54 insertions(+), 5 deletions(-) > > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c > index a4fea7d..b97c766 100644 > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.c > @@ -311,11 +311,43 @@ lp_build_emit_fetch( > } > > if (reg->Register.Absolute) { > - res = lp_build_emit_llvm_unary(bld_base, TGSI_OPCODE_ABS, res); > + switch (stype) { > + case TGSI_TYPE_FLOAT: > + case TGSI_TYPE_DOUBLE: > + case TGSI_TYPE_UNTYPED: > + /* modifiers on movs assume data is float */ > + res = lp_build_emit_llvm_unary(bld_base, TGSI_OPCODE_ABS, res); > + break; > + case TGSI_TYPE_UNSIGNED: > + case TGSI_TYPE_SIGNED: > + case TGSI_TYPE_VOID: > + default: > + /* abs modifier is only legal on floating point types */ > + assert(0); > + break; > + } > } > > if (reg->Register.Negate) { > - res = lp_build_negate( &bld_base->base, res ); > + switch (stype) { > + case TGSI_TYPE_FLOAT: > + case TGSI_TYPE_UNTYPED: > + /* modifiers on movs assume data is float */ > + res = lp_build_negate( &bld_base->base, res ); > + break; > + case TGSI_TYPE_DOUBLE: > + /* no double build context */ > + assert(0); > + break; > + case TGSI_TYPE_SIGNED: > + res = lp_build_negate( &bld_base->int_bld, res ); > + break; > + case TGSI_TYPE_UNSIGNED: > + case TGSI_TYPE_VOID: > + default: > + assert(0); > + break; > + } > } > > /* > diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c > b/src/gallium/auxiliary/tgsi/tgsi_exec.c > index 03f1942..1099d06 100644 > --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c > +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c > @@ -4150,7 +4150,7 @@ exec_instruction( > break; > > case TGSI_OPCODE_UADD: > - exec_vector_binary(mach, inst, micro_uadd, TGSI_EXEC_DATA_UINT, > TGSI_EXEC_DATA_UINT); > + exec_vector_binary(mach, inst, micro_uadd, TGSI_EXEC_DATA_INT, > TGSI_EXEC_DATA_INT); > break; > > case TGSI_OPCODE_UDIV: > diff --git a/src/gallium/auxiliary/tgsi/tgsi_info.c > b/src/gallium/auxiliary/tgsi/tgsi_info.c > index f289ebc..9c6fdfc 100644 > --- a/src/gallium/auxiliary/tgsi/tgsi_info.c > +++ b/src/gallium/auxiliary/tgsi/tgsi_info.c > @@ -277,9 +277,9 @@ tgsi_opcode_infer_src_type( uint opcode ) > case TGSI_OPCODE_AND: > case TGSI_OPCODE_OR: > case TGSI_OPCODE_XOR: > + /* XXX some src args may be signed for SAD ? */ > case TGSI_OPCODE_SAD: > case TGSI_OPCODE_U2F: > - case TGSI_OPCODE_UADD: > case TGSI_OPCODE_UDIV: > case TGSI_OPCODE_UMOD: > case TGSI_OPCODE_UMAD: > @@ -310,6 +310,8 @@ tgsi_opcode_infer_src_type( uint opcode ) > case TGSI_OPCODE_IABS: > case TGSI_OPCODE_ISSG: > case TGSI_OPCODE_UARL: > + /* UADD is both signed and unsigned require signed for working modifiers > */ > + case TGSI_OPCODE_UADD: > return TGSI_TYPE_SIGNED; > default: > return TGSI_TYPE_FLOAT; > @@ -331,7 +333,6 @@ tgsi_opcode_infer_dst_type( uint opcode ) > case TGSI_OPCODE_OR: > case TGSI_OPCODE_XOR: > case TGSI_OPCODE_SAD: > - case TGSI_OPCODE_UADD: > case TGSI_OPCODE_UDIV: > case TGSI_OPCODE_UMOD: > case TGSI_OPCODE_UMAD: > @@ -362,6 +363,7 @@ tgsi_opcode_infer_dst_type( uint opcode ) > case TGSI_OPCODE_ARR: > case TGSI_OPCODE_IABS: > case TGSI_OPCODE_ISSG: > + case TGSI_OPCODE_UADD: > return TGSI_TYPE_SIGNED; > default: > return TGSI_TYPE_FLOAT; > diff --git a/src/gallium/docs/source/tgsi.rst > b/src/gallium/docs/source/tgsi.rst > index dd4c773..d9a7fe9 100644 > --- a/src/gallium/docs/source/tgsi.rst > +++ b/src/gallium/docs/source/tgsi.rst > @@ -23,6 +23,21 @@ When an instruction has a scalar result, the result is > usually copied into > each of the components of *dst*. When this happens, the result is said to be > *replicated* to *dst*. :opcode:`RCP` is one such instruction. > > +Modifiers > +^^^^^^^^^^^^^^^ > + > +TGSI supports modifiers on inputs (as well as saturate modifier on > instructions). > + > +For inputs which have a floating point type, both absolute value and > negation > +modifiers are supported (with absolute value being applied first). > +TGSI_OPCODE_MOV is considered to have float input type for applying > modifiers. > + > +For inputs which have signed type only the negate modifier is supported. > This > +includes instructions which are otherwise ignorant if the type is signed or > +unsigned, such as TGSI_OPCODE_UADD. > + > +For inputs with unsigned type no modifiers are allowed. > + > Instruction Set > --------------- > > -- > 1.7.9.5 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev