On Thu, Dec 6, 2012 at 1:21 PM, Vincent Lejeune <v...@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 <t...@stellard.net> >> À : Michel Dänzer <mic...@daenzer.net> >> Cc : Vincent Lejeune <v...@ovi.com>; mesa-dev@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 <v...@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@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
