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

[Vincent Lejeune]

Sorry for the inconvenience.
I think the r600g backend work because of this patch, which switch MUL and MUL_IEEE definition :
http://lists.freedesktop.org/archives/mesa-dev/2012-November/030748.html

The rationale behind the patch is use the fmul instead of a custom intrinsic for OpenGL path.
Glsl-to-llvm generates as "vanilla" instructions as possible and uses fmul, this consolidates behaviour 
between tgsi-to-llvm and glsl-to-llvm. 
IMHO emitting MUL and MUL_IEEE should be decided with the presence of some "fast-math" arg to the backend,
or some context variable telling if we have a glsl or an opencl shader.

Vincent

----- Mail original -----
> De : Tom Stellard <tom at stellard.net>
> À : Michel Dänzer <michel at daenzer.net>
> Cc : Vincent Lejeune <vljn at ovi.com>; mesa-dev at lists.freedesktop.org
> Envoyé le : Jeudi 6 décembre 2012 18h05
> Objet : Re: [Mesa-dev] Mesa (master): r600g: Use default mul/mad function for tgsi-to-llvm
> 
> 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.
>> 
> 
> We really need to fix the SI AsmPrinter and start using the FileCheck tests
> in LLVM for things like this.
> 
> 
>>  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. 
> 
>>  BTW, some general issues with this commit:
>> 
>>  The prefix 'r600g:' is misleading, as this change affects radeonsi 
> as
>>  well. I think we've usually used 'radeon/llvm:' for changes
>>  src/gallium/drivers/radeon.
>> 
>>  There is no rationale for this change in the commit log: What is the
>>  intended effect? Why is it a good idea? ...
>> 
>> 
> 
> I agree with you here.  These are things I should have caught during the review.
> Should we revert this and the associated LLVM changes?
> 
> -Tom
> 
>>  -- 
>>  Earthling Michel Dänzer           |                  http://www.amd.com
>>  Libre software enthusiast         |          Debian, X and DRI developer
>>  _______________________________________________
>>  mesa-dev mailing list
>>  mesa-dev at lists.freedesktop.org
>>  http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>