Bug 91970

Mark Janes is bisecting.

I bisected to this to 

1e4e17fbd9296cc5064aabdb351a894d10190cb6
i965/fs: Lower integer multiplication after optimizations.

I noticed that the test did not fail reliably, so I will repeat the bisect.

Matt, does the commit seems like a plausible source of this failure?

Yes, that seems very plausible.

Created attachment 118317 [details] [review]
patch

The relevant instructions generated are

mul(8)          g124<1>D        g6<8,8,1>D      g10<8,8,1>UW    { align1 1Q };
mul(8)          g7<1>D          g6<8,8,1>D      g10<8,8,1>UW    { align1 1Q };
...
add(8)          g124.1<2>UW     g124.1<16,8,2>UW g7<16,8,2>UW   { align1 1Q };

I could imagine that the D * UW multiplies don't work as expected (maybe we should do D * W for signed multiplication?). Attached is a patch that gives that a try.

My second bisect confirmed the commit that regressed this test.  I am testing Matt's patch.

BTW, that patch doesn't apply cleanly to 10.6.

The patch provided breaks a bunch of tests, and doesn't fix the one listed in the bug.

(In reply to Matt Turner from comment #4)
> Created attachment 118317 [details] [review] [review]
> patch
> 
> The relevant instructions generated are
> 
> mul(8)          g124<1>D        g6<8,8,1>D      g10<8,8,1>UW    { align1 1Q
> };
> mul(8)          g7<1>D          g6<8,8,1>D      g10<8,8,1>UW    { align1 1Q

We're missing a .1 subreg offset on one of these.

> };
> ...
> add(8)          g124.1<2>UW     g124.1<16,8,2>UW g7<16,8,2>UW   { align1 1Q
> };
> 
> I could imagine that the D * UW multiplies don't work as expected (maybe we
> should do D * W for signed multiplication?). Attached is a patch that gives
> that a try.

If you enable the imul lowering on BDW, it fails there too.

g10 is from from ATTR file and we dont respect the subreg offset and stride for those.

Created attachment 118418 [details] [review]
Fix

We need to carry the ATTR stride and subreg information over to the hw reg when we assign hw regs to attributes.

Pushed to master:

commit 2ea16966ae66d4dd5c5dcb996d7996d9c734bbee
Author: Kristian Høgsberg Kristensen <krh@bitplanet.net>
Date:   Wed Sep 23 16:57:47 2015 -0700

    i965: Respect stride and subreg_offset for ATTR registers
    
    When we assign hw regs to attributes, we don't incorporate the stride
    and subreg_offset from the fs_reg. It's rarely used, but the integer
    multiplication lowering uses unusual stride and subreg_offset
    combination breaks when one source is an attribute.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91970
    Cc: "10.6 11.0" <mesa-stable@lists.freedesktop.org>
    Signed-off-by: Kristian Høgsberg Kristensen <krh@bitplanet.net>
    Reviewed-by: Matt Turner <mattst88@gmail.com>

 src/mesa/drivers/dri/i965/brw_fs.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)