Bug 31690

Summary: i915 shader compiler fails to flatten if in Aquarium webgl demo.
Product: Mesa Reporter: Kenneth Waters <kwaters>
Component: Drivers/DRI/i915Assignee: Kenneth Graunke <kenneth>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: git   
Hardware: x86 (IA32)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:

Description Kenneth Waters 2010-11-17 14:48:24 UTC
The demo can be located here, http://webglsamples.googlecode.com/hg/aquarium/aquarium.html .  The compiler fails to flatten an if statement in a fragment shader containing a discard.

uniform vec4 lightColor;
varying vec4 v_position;
varying vec2 v_texCoord;
varying vec3 v_normal;
varying vec3 v_surfaceToLight;
varying vec3 v_surfaceToView;
uniform vec4 ambient;
uniform sampler2D diffuse;
uniform vec4 specular;
uniform float shininess;
uniform float specularFactor;
uniform float fogPower;
uniform float fogMult;
uniform float fogOffset;
uniform vec4 fogColor;
vec4 lit(in float l, in float h, in float m){
return vec4(1.0, max(l, 0.0), (((l > 0.0)) ? (pow(max(0.0, h), m)) : (0.0)), 1.0);
}
void main(){
vec4 diffuseColor = texture2D(diffuse, v_texCoord);
if ((diffuseColor[3] < 0.30000001))
{
discard;
}
vec3 normal = normalize(v_normal);
vec3 surfaceToLight = normalize(v_surfaceToLight);
vec3 surfaceToView = normalize(v_surfaceToView);
vec3 halfVector = normalize((surfaceToLight + surfaceToView));
vec4 litR = lit(dot(normal, surfaceToLight), dot(normal, halfVector), shininess);
vec4 outColor = vec4((lightColor * (((diffuseColor * litR[1]) + (diffuseColor * ambient)) + ((specular * litR[2]) * specularFactor))).xyz, diffuseColor[3]);
(outColor = mix(outColor, vec4(fogColor.xyz, diffuseColor[3]), clamp(((pow((v_position[2] / v_position[3]), fogPower) * fogMult) - fogOffset), 0.0, 1.0)));
(gl_FragColor = outColor);
}

I've attached a full log with INTEL_DEBUG=wm and MESA_GLSL=dump, the shader in question is program 100, shader 102.
Comment 1 Eric Anholt 2010-11-17 17:22:46 UTC
Ken had talked about doing this (ir_discard taking a condition), so I'll assign it to him.  Several discard-related piglit tests already cover the issue.
Comment 2 Kenneth Graunke 2010-12-01 15:23:11 UTC
This should be fixed by the following commits:

commit 1802cb9bafc4125300870be51e8b22ddd795d61e
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Mon Nov 29 10:59:16 2010 -0800

    glsl: Remove "discard" support from lower_jumps.
    
    The new lower_discard and opt_discard_simplification passes should
    handle all the necessary transformations, so lower_jumps doesn't need to
    support it.
    
    Also, lower_jumps incorrectly handled conditional discards - it would
    unconditionally truncate all code after the discard.  Rather than fixing
    the bug, simply remove the code.
    
    NOTE: This is a candidate for the 7.9 branch.

commit 940df10100d740ef27fa39026fd51c3199ed3d62
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Thu Nov 25 01:09:26 2010 -0800

    glsl: Add a lowering pass to move discards out of if-statements.
    
    This should allow lower_if_to_cond_assign to work in the presence of
    discards, fixing bug #31690 and likely #31983.
    
    NOTE: This is a candidate for the 7.9 branch.

commit 9a1d063c6d679c2155f5eb80f1cb94368d36bf2c
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Wed Nov 24 22:02:26 2010 -0800

    glsl: Add an optimization pass to simplify discards.
    
    NOTE: This is a candidate for the 7.9 branch.

Technically I think only the middle commit is strictly necessary, but having all three is best.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.