[Mesa-dev] [PATCH] gallivm/tgsi: fix src modifier fetching with non-float types.
Jose Fonseca
jfonseca at vmware.com
Fri Feb 15 08:10:13 PST 2013
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 <sroland at 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
> >>
>
More information about the mesa-dev
mailing list