[Mesa-dev] [PATCH v2 09/20] i965/fs: indirect addressing with doubles is not supported in IVB/BYT

[Samuel Iglesias Gonsálvez]

On Fri, 2017-01-20 at 13:41 -0800, Matt Turner wrote:
> On Tue, Jan 17, 2017 at 1:49 AM, Samuel Iglesias Gonsálvez
> <siglesias at igalia.com> wrote:
> > It is tested empirically that IVB/BYT don't support indirect
> > addressing
> > with doubles but it is not documented in the PRM.
> > 
> > This patch applies the same solution than for Cherryview/Broxton
> > and
> > takes into account that we cannot double the stride, since the
> > hardware will do it internally.
> > 
> > v2:
> > - Fix assert to take into account Indirect DF MOVs in IVB and HSW.
> > 
> > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > ---
> 
> These two tests uncover a bug in this patch
> 
> spec/arb_gpu_shader_fp64/execution/fs-indirect-temp-double-const-src
> spec/arb_gpu_shader_fp64/uniform_buffers/fs-double-uniform-array-
> direct-indirect
> 
> They generate
> 
> add(8)          a0<1>UW         g6<4,4,1>UW     0x0070UW        {
> align1 1Q };
> mov(8)          g10<1>UD        g[a0]<VxH,1,0>UD                {
> align1 1Q };
> add(8)          a0<1>UW         g6.8<4,4,1>UW   0x0070UW        {
> align1 2N };
> mov(8)          g7<1>UD         g[a0]<VxH,1,0>UD                {
> align1 2N };
> add(8)          a0<1>UW         g8<4,4,1>UW     0x0070UW        {
> align1 1Q };
> mov(8)          g10.1<1>UD      g[a0]<VxH,1,0>UD                {
> align1 1Q };
>         ERROR: Writes must be evenly split between the two
> destination registers
> add(8)          a0<1>UW         g8.8<4,4,1>UW   0x0070UW        {
> align1 2N };
> mov(8)          g7.1<1>UD       g[a0]<VxH,1,0>UD                {
> align1 2N };
>         ERROR: Writes must be evenly split between the two
> destination registers
> 
> I think the mov(8)s from g[a0]<VxH,1,0>UD are supposed to be mov(4).
> 

Right.

This bug is not present in our -rc3 branch. However I discovered that,
although piglit was saying the test passed with the -auto flag, this
was not true. It was checking the color of one pixel that turned out to
be green, but some of the others are yellow (which is the color set by
the test for failure).

I am taking a look at this to provide a proper patch.

Sam

> I would have thought that lower_simd_width was the appropriate place
> to split SHADER_OPCODE_MOV_INDIRECT, rather than directly in the
> translation from NIR (as in commit bdab572a8). Maybe if that were
> fixed the solution to this problem would be somewhat simpler?
> 
> Curro, what do you think?
>