Bug 33197

Summary: [GLSL bisected] post increment (f++) on varying variable fails
Product: Mesa Reporter: Gordon Jin <gordon.jin>
Component: glsl-compilerAssignee: Eric Anholt <eric>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: high CC: idr, xunx.fang
Version: gitKeywords: regression
Hardware: All   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: new piglit test case

Description Gordon Jin 2011-01-16 23:41:32 UTC
Created attachment 42112 [details]
new piglit test case

The attached shader fails on mesa master (with both swrast and i965):
Probe at (0,0)
   Expected: 0.500000,0.000000,0.000000,1.000000
   Observed: 1.000000,0.000000,0.000000,1.000000

It passes if not using post increment (++).
It passes if removing declaration "varying float f" in the fragment shader.

Bisect shows 34a9da4eb4dd41dc874f1a175e993e42d4ff4b2a is the first bad commit Author: Eric Anholt <eric@anholt.net>
Date:   Wed Jan 12 18:25:33 2011 -0800

    mesa: Add channel-wise copy propagation to ir_to_mesa.
    
    This catches more opportunities than the prog_optimize.c code on
    openarena's fixed function shaders turned to GLSL, mostly due to
    looking at multiple source instructions for copy propagation
    opportunities.  It should also be much more CPU efficient than
    prog_optimize.c's code.
Comment 1 Ian Romanick 2011-01-25 18:30:13 UTC
I added glsl-vs-post-increment-01 (based on attachment #42112 [details]) and glsl-fs-post-increment-01 to reproduce this issue.  These tests are a bit simplified versus the attachment.  It seems that the function call is necessary, but separate compilation units is not.

The Mesa IR generated for the vertex shader in glsl-vs-post-increment-01 looks correct (see below).  However, I think this dump occurs before the new copy propagation pass.

Mesa IR for linked vertex program 3:
  0: (assign  (xyzw) (var_ref gl_Position@0x141f750)  (var_ref gl_Vertex@0x16aa740) ) 
     MOV OUTPUT[0], INPUT[0];
  1: (assign  (x) (var_ref f@0x13adfe0)  (constant float (0.500000)) ) 
     MOV OUTPUT[16], CONST[0].xxxx;
  2: (assign  (x) (var_ref _post_incdec_tmp@0x12e34c0)  (var_ref f@0x13adfe0) ) 
     MOV TEMP[1], OUTPUT[16].xxxx;
  3: (assign  (x) (var_ref f@0x13adfe0)  (constant float (1.500000)) ) 
     MOV OUTPUT[16], CONST[0].yyyy;
  4: (assign  (y) (var_ref color@0x13ba560)  (var_ref _post_incdec_tmp@0x12e34c0) ) 
     MOV OUTPUT[17].y, OUTPUT[16].xxxx;
  5: (assign  (xzw) (var_ref color@0x13ba560)  (constant vec3 (0.000000 0.000000 0.000000)) ) 
     MOV OUTPUT[17].xzw, CONST[1].xxyz;
  6: END

The output of INTEL_DEBUG=vs is, however, bogus (see below).  Line 3 should use CONST[0].xxxx, not OUTPUT[16].xxxx.

vs-mesa:
# Vertex Program/Shader 3
  0: MOV OUTPUT[0], INPUT[0];
  1: MOV OUTPUT[16], CONST[0].xxxx;
  2: MOV OUTPUT[16], CONST[0].yyyy;
  3: MOV OUTPUT[17].y, OUTPUT[16].xxxx;
  4: MOV OUTPUT[17].xzw, CONST[1].xxyz;
  5: END

brw_vs_alloc_regs NumAddrRegs 0
brw_vs_alloc_regs NumTemps 2
brw_vs_alloc_regs reg = 7
vs-native:
mov(8)          g3<1>F          g2<4,4,1>F                      { align16 };
mov(8)          g6<1>F          0.5F                            { align16 };
mov(8)          m8<1>F          g6<4,4,1>F                      { align16 };
mov(8)          g6<1>F          1.5F                            { align16 };
mov(8)          m8<1>F          g6<4,4,1>F                      { align16 };
mov(8)          m9<1>.yF        g6<4,4,1>F                      { align16 };
mov(8)          m9<1>.xzwF      g1.4<0,4,1>.xxyzF               { align16 };
send(8) 2       g7<1>F          g3<4,4,1>.wF
                math inv scalar mlen 1 rlen 1                   { align16 };
mul(8)          g7<1>.xyzF      g3<4,4,1>F      g7<4,4,1>F      { align16 };
mov(8)          m1<1>UD         0x00000000UD                    { align16 };
mov(8)          m2<1>F          g7<8,8,1>F                      { align1 };
mov(8)          m3<1>F          g3<8,8,1>F                      { align1 };
mov(8)          m7<1>F          g3<8,8,1>F                      { align1 };
send(8) 0       null            g0<8,8,1>F
                urb 0 urb_write interleave used complete mlen 10, rlen 0
 mlen 10 rlen 0 { align1 EOT };
Comment 2 Ian Romanick 2011-02-08 15:49:49 UTC
This should be fixed in master by the commit below.  Since the bug never existed in 7.9 or 7.10, I'm closing it now.

commit 76857e8954484d5bf8758d7bfc87f264f95a0ebd
Author: Eric Anholt <eric@anholt.net>
Date:   Fri Feb 4 13:31:02 2011 -0600

    mesa: Fix the Mesa IR copy propagation to not read past writes to the reg.
    
    Fixes glsl-vs-post-increment-01.
    
    Reviewed-by: José Fonseca <jfonseca@vmware.com>
Comment 3 Gordon Jin 2011-02-15 21:58:35 UTC
verified on Piketon.

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.