[Mesa-dev] [PATCH 4/4] i965: Properly handle integer types in opt_vector_float().

[Kenneth Graunke]

On Monday, April 18, 2016 11:50:53 AM PDT Matt Turner wrote:
> On Sun, Apr 17, 2016 at 11:14 PM, Kenneth Graunke <kenneth at whitecape.org> 
wrote:
> > Previously, opt_vector_float() always interpreted MOV sources as
> > floating point, and always created a MOV with a F-type destination.
> >
> > This meant that we could mess up sequences of integer loads, such as:
> >
> >    mov vgrf6.0.x:D, 0D
> >    mov vgrf6.0.y:D, 1D
> >    mov vgrf6.0.z:D, 2D
> >    mov vgrf6.0.w:D, 3D
> >
> > Here, integer 0/1/2/3 become approximately 0.0f, so we generated:
> >
> >    mov vgrf6.0:F, [0F, 0F, 0F, 0F]
> >
> > which is clearly wrong.  We can properly handle this by converting
> > integer values to float (rather than bitcasting), and emitting a type
> > converting MOV:
> >
> >    mov vgrf6.0:D, [0F, 1F, 2F, 3F]
> >
> > To do this, see first see if the integer values (converted to float)
> > are representable.  If so, we use a D-type MOV.  If not, we then try
> > the floating point values and an F-type MOV.  We make zero not impose
> > type restrictions.  This is important because 0D would imply a D-type
> > MOV, but is often used in sequences such as MOV 0D, MOV 0x3f800000D,
> > where we want to use an F-type MOV.
> >
> > Fixes about 54 dEQP-GLES2 failures with the vec4 VS backend.  This
> > recently became visible due to changes in opt_vector_float() which
> > made it optimize more cases, but it was a pre-existing bug.
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> 
> Hurts a single program in shader-db... for some reason related to
> seeing a zero first?
> 
> In toki-tori-2/1, we see
> 
> -mov(8)          g18<1>.zwF      [0F, 0F, 0F, 1F]VF
> +mov(8)          g18<1>.zUD      0x00000000UD
> +mov(8)          g18<1>.wD       1065353216D
> 
> Ignore the UD type -- the generator changes D -> UD so it can compact
> the instruction. It's actually type-D when opt_vector_float is called.

Thanks...this was a bug.  The larger code sequence was:

    mov vgrf13.0.x:D, 1D
    mov vgrf5.0.z:D, 0D
    mov vgrf5.0.w:D, 1065353216D

When we arrive at the second instruction, inst_count > 0 and dest_type
is D (from the first instruction).  We try to avoid adding requirements
to the type by setting need_type to dest_type, but that's actually the
left over type from the previous sequence.  We then flush, reset
dest_type to F.  We then record the second instruction, setting
dest_type to need_type, which was incorrectly D.  Processing the third
instruction sees that dest_type (D) isn't equal to need_type (F), so it
flushes, which just bails since there isn't anything to batch up.

Nasty.  At any rate, I think I have a +5 -8 LOC fix.  Testing it now.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160418/031e32f7/attachment.sig>