Bug 107193

Summary: piglit.spec.arb_compute_shader.linker.bug-93840 fails
Product: Mesa Reporter: Mark Janes <mark.a.janes>
Component: Drivers/DRI/i965Assignee: Chema Casanova <jmcasanova>
Status: RESOLVED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium CC: caio.oliveira
Version: gitKeywords: bisected, regression
Hardware: Other   
OS: All   
i915 platform: i915 features:

Description Mark Janes 2018-07-11 15:22:45 UTC

Standard Output

/tmp/build_root/m64/lib/piglit/bin/shader_runner /tmp/build_root/m64/lib/piglit/tests/spec/arb_compute_shader/linker/bug-93840.shader_test -auto -fbo

Standard Error

shader_runner: ../src/intel/compiler/brw_fs_generator.cpp:2455: int fs_generator::generate_code(const cfg_t*, int): Assertion `validated' failed.
Comment 1 Mark Janes 2018-07-11 15:23:14 UTC
0e47ecb29acf8bdd213236d7306c47f8ec0e937f is the first bad commit
commit 0e47ecb29acf8bdd213236d7306c47f8ec0e937f
Author: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
Date:   Mon Mar 26 14:59:46 2018 +0200
    intel/compiler: grf127 can not be dest when src and dest overlap in send
    Implement at brw_eu_validate the restriction from Intel Broadwell PRM,
    vol 07, section "Instruction Set Reference", subsection "EUISA
    Instructions", Send Message (page 990):
    "r127 must not be used for return address when there is a src and
    dest overlap in send instruction."
    v2: Style fixes (Matt Turner)
    Reviewed-by: Matt Turner <mattst88@gmail.com>
    Cc: 18.1 <mesa-stable@lists.freedesktop.org>
Comment 2 Mark Janes 2018-07-11 15:32:13 UTC
patch posted:
Comment 3 Chema Casanova 2018-07-11 16:54:00 UTC
I've just posted a patch to deal with this new case that new validation rule detects:

Comment 4 Chema Casanova 2018-07-12 16:13:55 UTC
Fixed by:

commit 62f37ee53d9d5388eecef94369893b5467349306
Author: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
Date:   Wed Jul 11 11:19:20 2018 +0200

    i965/fs: unspills shoudn't use grf127 as dest since Gen8+
    At 232ed8980217dd65ab0925df28156f565b94b2e5 "i965/fs: Register allocator
    shoudn't use grf127 for sends dest" we didn't take into account the case
    of SEND instructions that are not send_from_grf. But since Gen7+ although
    the backend still uses MRFs internally for sends they are finally
    assigned to a GRFs.
    In the case of unspills the backend assigns directly as source its
    destination because it is suppose to be available. So we always have a
    source-destination overlap. If the reg_allocator assigns registers that
    include the grf127 we fail the validation rule that affects Gen8+
    "r127 must not be used for return address when there is a src and dest
    overlap in send instruction."
    So this patch activates the grf127_send_hack_node for Gen8+ and if we
    have any register spilled we add interferences to the destination of
    the unspill operations.
    We also need to avoid that opt_bank_conflicts() optimization, that runs
    after the register allocation, doesn't move things around, causing the
    grf127 to be used in the condition we were avoiding.
    Fixes piglit test tests/spec/arb_compute_shader/linker/bug-93840.shader_test
    and some shader-db crashed because of the grf127 validation rule..
    v2: make sure that opt_bank_conflicts() optimization doesn't change
    the use of grf127. (Caio)
    Found by Caio Marcelo de Oliveira Filho
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107193
    Fixes: 232ed89802 "i965/fs: Register allocator shoudn't use grf127 for sends dest"
    Cc: 18.1 <mesa-stable@lists.freedesktop.org>
    Cc: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
    Cc: Jason Ekstrand <jason@jlekstrand.net>
    Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.