Bug 110825

Summary: valgrind invalid reads when spilling
Product: Mesa Reporter: Tapani Pälli <lemody>
Component: Drivers/DRI/i965Assignee: Jason Ekstrand <jason>
Status: RESOLVED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium CC: eero.t.tamminen
Version: git   
Hardware: Other   
OS: All   
See Also: https://bugs.freedesktop.org/show_bug.cgi?id=93681
Whiteboard:
i915 platform: i915 features:
Attachments: valgrind output

Description Tapani Pälli 2019-06-03 11:47:46 UTC
According to valgrind output there seems to be invalid reads when registers gets spilled. I've bisected this to start happening with following commit:

b2d274c6775 intel/fs/ra: Choose a spill reg before throwing away the graph
Comment 1 Tapani Pälli 2019-06-03 11:48:07 UTC
Created attachment 144425 [details]
valgrind output
Comment 2 Tapani Pälli 2019-06-03 11:49:17 UTC
Assigning to Jason as it is his commit.
Comment 4 Tapani Pälli 2019-06-05 05:35:50 UTC
commit a84de3fb7c1198f7cbd3b20a4231c14a7010f97f
Author: Jason Ekstrand <jason@jlekstrand.net>
Date:   Mon Jun 3 17:09:12 2019 -0500

    intel/fs: Skip registers faster when setting spill costs
    
    This might be slightly faster since we're doing one read rather than
    two before we decide to skip.  The more important reason, however, is
    because no_spill prevents us from re-spilling spill registers.  In the
    new world in which we don't re-calculate liveness every spill, we may
    not have valid liveness for spill registers so we shouldn't even look
    their live ranges up.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110825
    Fixes: e99081e76d4 "intel/fs/ra: Spill without destroying the..."
    Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira@intel.com>
    Tested-by: Tapani Pälli <tapani.palli@intel.com>
Comment 5 Tapani Pälli 2019-06-07 06:04:46 UTC
FYI now that I tested with KBL, I see following error/warning. I do see it before the offending commit though so this is something that has existed before. For some reason I did not spot these in HSW. That test eventually crashes on HSW but passes on KBL with this single warning.

--- 8< ---
==12320== Memcheck, a memory error detector
==12320== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==12320== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==12320== Command: bin/shader_runner /home/tpalli/slow.shader_test -auto
==12320== 
==12320== Conditional jump or move depends on uninitialised value(s)
==12320==    at 0xC2A36FC: ra_get_best_spill_node (register_allocate.c:960)
==12320==    by 0xBF99499: fs_reg_alloc::choose_spill_reg() (brw_fs_reg_allocate.cpp:937)
==12320==    by 0xBF9A0A8: fs_reg_alloc::assign_regs(bool, bool) (brw_fs_reg_allocate.cpp:1161)
==12320==    by 0xBF9A3AA: fs_visitor::assign_regs(bool, bool) (brw_fs_reg_allocate.cpp:1201)
==12320==    by 0xBF619A7: fs_visitor::allocate_registers(unsigned int, bool) (brw_fs.cpp:7282)
==12320==    by 0xBF62D20: fs_visitor::run_fs(bool, bool) (brw_fs.cpp:7694)
==12320==    by 0xBF63CC8: brw_compile_fs (brw_fs.cpp:8028)
==12320==    by 0xB7BCE62: brw_codegen_wm_prog (brw_wm.c:123)
==12320==    by 0xB7BE25D: brw_fs_precompile (brw_wm.c:592)
==12320==    by 0xB7A423C: brw_shader_precompile(gl_context*, gl_shader_program*) (brw_link.cpp:56)
==12320==    by 0xB7A541F: brw_link_shader (brw_link.cpp:374)
==12320==    by 0xBAC983D: _mesa_glsl_link_shader (ir_to_mesa.cpp:3170)
==12320==

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.