mesa: 58b386dce435d2d82c2dc80b1a8d1373bb3e0ac6 (master 10.4.0-devel)
piglit glsl-vs-clamp-1 times out on swrast.
8f890b119eaff88a7fad64abbf183cbcc22edc7a is the first bad commit
Author: Abdiel Janulgue <email@example.com>
Date: Thu Jun 19 22:15:14 2014 -0700
glsl: Optimize clamp(x, 0, 1) as saturate(x)
v2: - Check that the base type is float (Ian Romanick)
v3: - Make sure comments reflect that we are doing a commutative operation
- Add missing condition where the inner constant is 1.0 and outer constant is 0.0
- Make indexing of operands easier to read (Matt Turner)
Reviewed-by: Matt Turner <firstname.lastname@example.org>
Reviewed-by: Ian Romanick <email@example.com>
Signed-off-by: Abdiel Janulgue <firstname.lastname@example.org>
:040000 040000 5174a30266aa05b4334c831e1cd51de1376e4b70 916e9faa0afc6c04e530af2a3c0dfb938a7f07de M src
bisect run success
We got an infinite loop in swrast which is caused by the back and forth ping-pong between saturate-to-clamp lowering pass and the clamp-to-saturate opt_algebraic optimization pass. I got a fix coming up which optionally disables the opt_algebraic optimization by adding an EmitNoSat toggle in gl_shader_compiler_options.
Created attachment 105892 [details] [review]
Vinson, can you please try the attached patch if it fixes the issue?
swrast should be able to handle ir_unop_saturate just fine - it shouldn't need the lowering pass at all, even in the vertex shader.
This may still be necessary though - I don't know if r200 can do this (or if it even supports GLSL), or about pre-SM3 gallium drivers.
It sounds like drivers should set options->EmitNoSat for the VS stages, and we should only add the SAT_TO_CLAMP flag when options->EmitNoSat is set (in addition to disabling the opt_algebraic optimization in that case). That way, saturate actually happens for drivers that can support it.
attachment 105892 [details] [review] fixes swrast regression.
Tested-by: Vinson Lee <email@example.com>
These other piglit regressions were also introduced with commit 8f890b119eaff88a7fad64abbf183cbcc22edc7a.
(In reply to Kenneth Graunke from comment #3)
> swrast should be able to handle ir_unop_saturate just fine - it shouldn't
> need the lowering pass at all, even in the vertex shader.
> This may still be necessary though - I don't know if r200 can do this (or if
> it even supports GLSL), or about pre-SM3 gallium drivers.
> It sounds like drivers should set options->EmitNoSat for the VS stages, and
> we should only add the SAT_TO_CLAMP flag when options->EmitNoSat is set (in
> addition to disabling the opt_algebraic optimization in that case). That
> way, saturate actually happens for drivers that can support it.
Hi Ken! Sorry for the delay. I was not yet sure which drivers you mean. Do you mean *all* drivers (including r200)? Or if I understood this correctly, I would have to set EmitNoSat in the gallium state tracker and in ir_to_mesa as is done in this patch?
*** Bug 83384 has been marked as a duplicate of this bug. ***
Patch has now landed in 10.4: