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

[Alex Deucher]

On Thu, Dec 6, 2012 at 1:21 PM, Vincent Lejeune <vljn at ovi.com> wrote:
> 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.

MUL and MUL_IEEE are not related to performance, rather to behavior:

MUL            Floating-point multiply. 0*anything = 0.
MUL_IEEE  IEEE Floating-point multiply. Uses IEEE rules for 0*anything.

Alex

>
> 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
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev