Bug 43218

Summary: [bisected SNB] piglit spec/glsl-1.10/execution/built-in-functions/fs-op-div-int-int fails
Product: Mesa Reporter: fangxun <xunx.fang>
Component: Drivers/DRI/i965Assignee: Ian Romanick <idr>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: high    
Version: git   
Hardware: All   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 42993    

Description fangxun 2011-11-24 01:49:05 UTC 
System Environment:
--------------------------
Arch:           i386
Platform:       huronriver
Libdrm:         (master)2.4.27-2-gca4971292cf99e0063416cd1c3467af94637bf2b
Mesa:           (master)94cd9d6be8980f28fdb6b7a8526814802a90f25d
Xserver:                (master)xorg-server-1.11.99.1
Xf86_video_intel:(master)2.17.0-10-g3b9479dc39d32fd97f80c1e5e0fac67d36ee5e40
Kernel:  (drm-intel-next)9a10f401a401ca69c6537641c8fc0d6b57b5aee8


Bug detailed description:
------------------------- 
It fails on Sandybridge. Below piglit cases also fail due to the same
cause. 
spec/glsl-1.10/execution/built-in-functions/fs-op-div-int-ivec4 
spec/glsl-1.10/execution/built-in-functions/fs-op-div-ivec2-ivec2 
spec/glsl-1.10/execution/built-in-functions/fs-op-div-ivec4-int 
spec/glsl-1.10/execution/built-in-functions/fs-op-div-ivec4-ivec4
spec/glsl-1.10/execution/built-in-functions/vs-op-div-int-int 
spec/glsl-1.10/execution/built-in-functions/vs-op-div-int-ivec4
spec/glsl-1.10/execution/built-in-functions/vs-op-div-ivec2-ivec2 
spec/glsl-1.10/execution/built-in-functions/vs-op-div-ivec4-int
spec/glsl-1.10/execution/built-in-functions/vs-op-div-ivec4-ivec4 

Bisect shows 143d20c16a33e2f08e834b28c23dbea772591ef9 is the first bad commit.
commit 143d20c16a33e2f08e834b28c23dbea772591ef9
Author:     Ian Romanick <ian.d.romanick@intel.com>
AuthorDate: Mon Oct 24 16:37:01 2011 -0700
Commit:     Ian Romanick <ian.d.romanick@intel.com>
CommitDate: Mon Nov 7 13:33:16 2011 -0800

    i965: Move _mesa_ir_link_shader call before device-specific linking

    _mesa_ir_link_shader needs to be called before cloning the IR tree so
    that the var->location field for uniforms is set.

    WARNING: This change breaks several integer division related piglit
    tests.  The tests break because _mesa_ir_link_shader lowers integer
    division to an RCP followed by a MUL.  The fix is to factor out more
    of the code from ir_to_mesa so that _mesa_ir_link_shader does not need
    to be called at all by the i965 driver.  This will be the subject of
    several follow-on patches.

    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Tested-by: Tom Stellard <thomas.stellard@amd.com>

Reproduce steps:
----------------
1. start X
2. ./shader_runner spec/glsl-1.10/execution/built-in-functions/fs-op-div-int-int.shader_test -auto -fbo
Comment 1 Ian Romanick 2012-01-12 11:34:46 UTC 
This should be fixed by the following commit in Mesa master (and 8.0 branch):

commit 1c177452005a0366db01629d875da553f7949ddd
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Fri Jan 6 16:26:49 2012 -0800

    i965: Don't use _mesa_ir_link_shader to do our dirty work
    
    Instead, do the uniform setting and input / output mapping directly in
    brw_link_shader.  Hurray for not generating Mesa IR!  However, once
    the i965 driver stops calling _mesa_ir_link_shader, UsesClipDistance
    and UsesKill are no longer set.
    
    Ideally gen6_upload_vs_push_constants should use the
    gl_shader_program, but I don't see a way to propagate the information
    there.  The other alternative, since this is the only usage, is to
    move gl_vertex_program::UsesClipDistance to brw_vertex_program.
    
    The compile (and precompile) stages use UsesKill to determine the
    cache key for the shader.  This is then used to determine whether or
    not to compile the shader.  Calculating this data during compilation
    is too late.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Acked-by: Kenneth Graunke <kenneth@whitecape.org>
    Acked-by: Eric Anholt <eric@anholt.net>
Comment 2 Ouping Zhang 2012-01-30 23:50:04 UTC 
System Environment:
--------------------------
Libdrm:         (master)2.4.30-18-gb643b0713aefdc0611e47654e88263b53b0de6f5
Mesa:           (8.0)caebd7929dca802ece8ef36b0f85094d66133b57
Xserver:                (server-1.11-branch)xorg-server-1.11.3
Xf86_video_intel:              
(master)2.17.0-599-gca252e5b51d7b2f5a7b2c2e0d8fdb024b08096db

verify with mesa 8.0 branch, this case passed on huronriver.

(In reply to comment #1)
> This should be fixed by the following commit in Mesa master (and 8.0 branch):
> commit 1c177452005a0366db01629d875da553f7949ddd
> Author: Ian Romanick <ian.d.romanick@intel.com>
> Date:   Fri Jan 6 16:26:49 2012 -0800
>     i965: Don't use _mesa_ir_link_shader to do our dirty work
>     Instead, do the uniform setting and input / output mapping directly in
>     brw_link_shader.  Hurray for not generating Mesa IR!  However, once
>     the i965 driver stops calling _mesa_ir_link_shader, UsesClipDistance
>     and UsesKill are no longer set.
>     Ideally gen6_upload_vs_push_constants should use the
>     gl_shader_program, but I don't see a way to propagate the information
>     there.  The other alternative, since this is the only usage, is to
>     move gl_vertex_program::UsesClipDistance to brw_vertex_program.
>     The compile (and precompile) stages use UsesKill to determine the
>     cache key for the shader.  This is then used to determine whether or
>     not to compile the shader.  Calculating this data during compilation
>     is too late.
>     Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
>     Acked-by: Kenneth Graunke <kenneth@whitecape.org>
>     Acked-by: Eric Anholt <eric@anholt.net>

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.