[Mesa-dev] [PATCH] r600g/sb: Don't fold integer value into float CND

[Connor Abbott]

What does the source look like? Both the TGSI and GLSL looks fine to
me. This is essentially doing:

vec3 vec_value_tmp = ...;
bvec3 dereference_condition = equals(row.xxx, vec3(0, 1, 2));
//per-component equals
float tmp; //called vec_index_tmp_v in the GLSL IR
// this is where the use-before-def comes in -- note this is a
conditional move in GLSL IR and UCMP in TGSI
tmp = dereference_condition.x ? vec_value_tmp.x : tmp;
tmp = dereference_condition.y ? vec_value_tmp.y : tmp;
tmp = dereference_condition.z ? vec_value_tmp.z : tmp;

which the compiler lowered from:

tmp = vec_value_tmp[row];

Note that while we do use tmp before we store it, we're guaranteed to
have a valid value for tmp by the time we reach the end since in the
original source code row was used as an index (and we can give
undefined results if row is out-of-range). So it seems like it's a
driver bug.


On Thu, Feb 12, 2015 at 11:30 PM, Dave Airlie <airlied at gmail.com> wrote:
> On 12 February 2015 at 23:49, Maarten Lankhorst
> <maarten.lankhorst at canonical.com> wrote:
>> Op 12-02-15 om 14:23 schreef Glenn Kennard:
>>> Don't try to do float comparisons on signed integer values,
>>> some of them look like NaNs.
>>>
>>> Fixes fs-temp-array-mat3-index-col-row-rd.shader_test regression
>>> caused by 0d4272cd8e7c45157140dc8e283707714a8238d5.
>>>
>>> Signed-off-by: Glenn Kennard <glenn.kennard at gmail.com>
>>>
>> Cc: "10.5" <mesa-stable at lists.freedesktop.org>
>> Reported-andTested-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
>
> This is certinaly right and leads to a problem later on though,
>
> tests/spec/glsl-1.10/execution/variable-indexing/fs-uniform-array-mat3-col-row-rd.shader_test
> regresses as pointed out,
>
> however the TGSI generated for this as an uninitialised variable in it
>
> FRAG
> PROPERTY FS_COLOR0_WRITES_ALL_CBUFS 1
> DCL OUT[0], COLOR
> DCL CONST[0..8]
> DCL TEMP[0..2], LOCAL
> DCL ADDR[0]
> IMM[0] INT32 {0, 1, 2, 0}
> IMM[1] FLT32 {    0.0000,     1.0000,     0.0000,     0.0000}
>   0: UARL ADDR[0].x, CONST[0].xxxx
>   1: MOV TEMP[0].xyz, CONST[ADDR[0].x+6].xyzx
>   2: USEQ TEMP[1].xyz, CONST[1].xxxx, IMM[0].xyzz
>   3: UCMP TEMP[2].x, TEMP[1].xxxx, TEMP[0].xxxx, TEMP[2].xxxx
>   4: UCMP TEMP[2].x, TEMP[1].yyyy, TEMP[0].yyyy, TEMP[2].xxxx
>   5: UCMP TEMP[2].x, TEMP[1].zzzz, TEMP[0].zzzz, TEMP[2].xxxx
>   6: FSEQ TEMP[0].x, TEMP[2].xxxx, CONST[2].xxxx
>   7: UIF TEMP[0].xxxx :0
>   8:   MOV TEMP[0], IMM[1].xyxy
>   9: ELSE :0
>  10:   MOV TEMP[0], IMM[1].yxxy
>  11: ENDIF
>  12: MOV OUT[0], TEMP[0]
>  13: END
>
> note TEMP[2] is used a src before being used a dst
>
> the GLSL for this
> GLSL IR for linked fragment program 3:
> (
> (declare (shader_out ) vec4 gl_FragColor)
> (declare (temporary ) vec4 gl_FragColor)
> (declare (uniform ) int col)
> (declare (uniform ) int row)
> (declare (uniform ) float expect)
> (declare (uniform ) (array mat3 2) m)
> (function main
>   (signature void
>     (parameters
>     )
>     (
>       (declare (temporary ) vec4 conditional_tmp)
>       (declare (temporary ) vec3 vec_value_tmp)
>       (assign  (xyz) (var_ref vec_value_tmp)  (array_ref (array_ref
> (var_ref m) (constant int (1)) ) (var_ref col) ) )
>       (declare (temporary ) float vec_index_tmp_v)
>       (declare (temporary ) bvec3 dereference_condition)
>       (assign  (xyz) (var_ref dereference_condition)  (expression
> bvec3 == (swiz xxx (var_ref row) )(constant ivec3 (0 1 2)) ) )
>       (assign (swiz x (var_ref dereference_condition) ) (x) (var_ref
> vec_index_tmp_v)  (swiz x (var_ref vec_value_tmp) ))
>       (assign (swiz y (var_ref dereference_condition) ) (x) (var_ref
> vec_index_tmp_v)  (swiz y (var_ref vec_value_tmp) ))
>       (assign (swiz z (var_ref dereference_condition) ) (x) (var_ref
> vec_index_tmp_v)  (swiz z (var_ref vec_value_tmp) ))
>       (if (expression bool == (var_ref vec_index_tmp_v) (var_ref expect) ) (
>         (assign  (xyzw) (var_ref conditional_tmp)  (constant vec4
> (0.000000 1.000000 0.000000 1.000000)) )
>       )
>       (
>         (assign  (xyzw) (var_ref conditional_tmp)  (constant vec4
> (1.000000 0.000000 0.000000 1.000000)) )
>       ))
>
>       (assign  (xyzw) (var_ref gl_FragColor)  (var_ref conditional_tmp) )
>       (assign  (xyzw) (var_ref gl_FragColor at 3)  (var_ref gl_FragColor) )
>     ))
>
> )
>
> I'm having trouble deciphering :-) and it comes from
> lower_vec_index_to_cond_assign.cpp.
>
> This is probably a glsl->tgsi convertor bug, any GLSL expert say if
> the GLSL is okay?
>
> Dave.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev