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

[Samuel Iglesias Gonsálvez]

On Mon, 2017-01-23 at 15:52 +0100, Samuel Iglesias Gonsálvez wrote:
> 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.
> 

I am discussing with Curro a good solution for this, however there are
still patches from this -rc2 that are still unreviewed. Would you
(either Curro or you) mind taking a look at them?

The idea is to fix more before sending v3 of the patch series.

Thanks!

Sam

> 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?
> > 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170201/b9e9191d/attachment-0001.sig>