[Mesa-dev] [PATCH] gallium/tgsi: clarify (possibly change) TGSI_OPCODE_UCMP definition

[Jose Fonseca]

----- Original Message -----
> The SUB opcode is not emitted by GLSL, however this may change in the
> future. I don't think using specialized opcodes instead of the
> modifiers will make anyone's life easier (the modifiers must be
> implemented anyway).
> 
> If you really want people to follow the TGSI documentation, there
> should be a utility function which validates TGSI and st/mesa should
> reject shaders which are invalid. Right now, GLSL-to-TGSI generates
> UCMP with the first operand negated.

I don't see why TGSI documentation is diminished without such tests.

I actually think such validation tests have limited use: it only tests the syntax of shaders -- not how drivers implement them --; and we actually need input that exercises those error conditions in runtime. Furthermore, it's more stuff to maintain -- just take a look at how graw/galhadad quickly rots.

No, I strongly believe that good gallium documentation is a much better legacy than more gallium specific tests.  For the rest we have piglit.

Jose

> 
> Marek
> 
> 
> 
> On Wed, May 8, 2013 at 3:51 PM, Roland Scheidegger <sroland at vmware.com>
> wrote:
> > Am 08.05.2013 12:47, schrieb Marek Olšák:
> >> Modifiers are actually very useful with MOV. However I don't think the
> >> modifiers really care what the type is. They just change the sign bit
> >> of float (which is a bitwise operation). Also, UCMP doesn't do
> >> anything with the 2nd and 3rd argument, so their types don't matter.
> > Well, they do matter for the input modifiers.
> > They might just change the most significant bit on your hardware but
> > that's not how they are defined in tgsi.
> > I don't really care that much if the 2nd and 3rd argument to ucmp are
> > ints or floats (it's true for d3d10 movc translation floats are the
> > natural choice but we can easily translate that to several ops instead,
> > e.g. mov's with modifiers followed by ucmp). I do care however that all
> > drivers treat them the same, not like now where softpipe will do two's
> > complement negation on src input modifiers whereas llvmpipe will flip
> > the sign bit.
> >
> >>
> >> I think the ABS and SUB opcodes should be removed in favor of the
> >> modifiers.
> > Those are in very widespread use and at least for SUB it definitely
> > avoids additional effort for drivers which can't do input modifiers
> > natively.
> >
> > Roland
> >
> >
> >>
> >> Marek
> >>
> >> On Wed, May 8, 2013 at 12:14 PM, Christoph Bumiller
> >> <e0425955 at student.tuwien.ac.at> wrote:
> >>> On 08.05.2013 03:48, sroland at vmware.com wrote:
> >>>> From: Roland Scheidegger <sroland at vmware.com>
> >>>>
> >>>> UCMP while an integer opcode isn't really consistently implemented as
> >>>> having all integer arguments. softpipe will assume all arguments are
> >>>> ints, whereas gallivm has the arguments defined as untyped which
> >>>> means they'll get treated as floats. This means input modifiers will
> >>>> not work the same. Fix this by saying only first arg is an integer,
> >>>> which seems more useful than making all arguments integers - this would
> >>>> be similar to d3d10 movc opcode.
> >>>> ---
> >>>>  src/gallium/docs/source/tgsi.rst |    5 +++++
> >>>>  1 file changed, 5 insertions(+)
> >>>>
> >>>> diff --git a/src/gallium/docs/source/tgsi.rst
> >>>> b/src/gallium/docs/source/tgsi.rst
> >>>> index 3af1fb7..852f8a0 100644
> >>>> --- a/src/gallium/docs/source/tgsi.rst
> >>>> +++ b/src/gallium/docs/source/tgsi.rst
> >>>> @@ -1291,6 +1291,11 @@ Support for these opcodes indicated by
> >>>> PIPE_SHADER_CAP_INTEGERS (all of them?)
> >>>>
> >>>>  .. opcode:: UCMP - Integer Conditional Move
> >>>>
> >>>> +.. note::
> >>>> +
> >>>> +   Only the first source arg is an integer, the 2nd and 3rd ones are
> >>>> +   considered floats (for input modifier purposes).
> >>>> +
> >>>
> >>> As long as you patch up all the occurrences of
> >>> tgsi_opcode_infer_src_type and make it take an argument to identify the
> >>> source ...
> >>>
> >>> I'd rather just forbid modifiers on moves, i.e. MOV and UCMP, since at
> >>> least MOV returns TGSI_TYPE_UNTYPED and untyped values can't be operated
> >>> on.
> >>> For the ordinary MOV we have NEG and ABS, and for UCMP the backend
> >>> optimizer can take care of merging modifiers into the instruction
> >>> (nvc0's UCMP (slct u32) doesn't support modifiers)
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>