Fair enough. I don't feel strongly either way. Jose
----- Original Message ----- > Allowing that looks very odd to me, and it seems no API does that. > FWIW the MOV with negation can't work like that (not in a consistent way > at least) since for ordinary temps etc. we don't know the type (hence > arguements are considered floats). > I think if you'd really want to do something like that you should just > use explicit negation rather than rely on modifiers. Negation on > unsigned numbers is imho a abuse (granted a well defined one and not > that rare) which should rather be explicit. > > Roland > > > Am 15.02.2013 16:32, schrieb Jose Fonseca: > > 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