[Mesa-dev] Mesa (master): r600g: Use default mul/mad function for tgsi-to-llvm

[Tom Stellard]

On Thu, Dec 06, 2012 at 07:25:55PM +0100, Michel Dänzer wrote:
> On Don, 2012-12-06 at 09:05 -0800, Tom Stellard wrote: 
> > On Thu, Dec 06, 2012 at 05:08:07PM +0100, Michel Dänzer wrote:
> > > On Mit, 2012-12-05 at 09:32 -0800, Vincent Lejeune wrote: 
> > > > Module: Mesa
> > > > Branch: master
> > > > Commit: 0ad1fefd6951aa47ab58a41dc9ee73083cbcf85c
> > > > URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=0ad1fefd6951aa47ab58a41dc9ee73083cbcf85c
> > > > 
> > > > Author: Vincent Lejeune <vljn at ovi.com>
> > > > Date:   Wed Nov 28 00:35:55 2012 +0100
> > > > 
> > > > r600g: Use default mul/mad function for tgsi-to-llvm
> > > 
> > > This change breaks the piglit tests glsl-{f,v}s-vec4-indexing-temp-src
> > > on radeonsi. I suspect the same would be true with r600g as well if it
> > > didn't fall back to the non-LLVM backend for this test.
> > > 
> > > Comparing the generated code, I've noticed two main differences so far:
> > > 
> > >       * LLVM now optimizes away some TGSI MUL operations with constant
> > >         1.0, which previously resulted in V_MUL_LEGACY_F32 (non-IEEE
> > >         semantics) instructions. 
> > >       * V_MUL_F32 and V_ADD_F32 (IEEE semantics) are used instead of
> > >         V_MAD_LEGACY_F32 (non-IEEE semantics) in some places.
> 
> BTW, I have a fix so the second bullet at least becomes V_MAD_F32.
> 
> 
> > > I suspect the problem is a (non-)IEEE semantics mismatch between TGSI
> > > and LLVM introduced by this change.
> > 
> > What are the semantics of TGSI opcodes?  For MUL and MAD, tgsi_exec uses IEEE
> > operations, but it seems like the glsl frontend thinks they are non-IEEE. 
> 
> Not sure... FWIW, the tests pass on llvmpipe.
>

I just looked at the TGSI->LLVM code, which is shared by r600g,
radeonsi, and llvmpipe.  For TGSI_OPCODE_MUL, it actually uses both
the IEEE and non-IEEE definitions depending on what the arguments are.
If one of the arguments to MUL is the compile time constant zero then it
uses the non-IEEE definition (0 * anything) = 0, otherwise it produces
a fmul instruction which has IEEE semantics.  This probably deserves
it's own thread, because I think newer versions of GLSL require IEEE,
but older versions and also ARB have different semantics.

 
> Reverting only the MAD related changes doesn't fix the problem.
> Reverting only the MUL related change triggers a shader compile failure
> in LLVM.
> 
> 
> > Should we revert this and the associated LLVM changes?
> 
> I guess it depends on the answer to the above question about the
> expected semantics.
> 
> Given that these tests seem to use indirect addressing, could it be that
> they just happened to pass by accident before?

Yeah, those tests must have been passing by accident.  Are there any other
failing tests?

-Tom
> 
> 
> -- 
> Earthling Michel Dänzer           |                   http://www.amd.com
> Libre software enthusiast         |          Debian, X and DRI developer