On Wed, Nov 1, 2017 at 9:24 AM, Tapani Pälli <tapani.pa...@intel.com> wrote:
> > > On 11/01/2017 04:55 PM, Jason Ekstrand wrote: > >> On Wed, Nov 1, 2017 at 7:53 AM, Jason Ekstrand <ja...@jlekstrand.net >> <mailto:ja...@jlekstrand.net>> wrote: >> >> On Fri, Oct 27, 2017 at 5:55 AM, Tapani Pälli >> <tapani.pa...@intel.com <mailto:tapani.pa...@intel.com>> wrote: >> >> Patch uses mem_ctx for allocation to ensure param array gets freed >> later, in blorp clear case this happens in >> blorp_params_get_clear_kernel. >> >> ==6164== 48 bytes in 1 blocks are definitely lost in loss record >> 61 of 193 >> ==6164== at 0x4C2EB6B: malloc (vg_replace_malloc.c:299) >> ==6164== by 0x12E31C6C: ralloc_size (ralloc.c:121) >> ==6164== by 0x130189F1: >> fs_visitor::assign_constant_locations() (brw_fs.cpp:2095) >> ==6164== by 0x13022D32: fs_visitor::optimize() >> (brw_fs.cpp:5715) >> ==6164== by 0x13024D5A: fs_visitor::run_fs(bool, bool) >> (brw_fs.cpp:6229) >> ==6164== by 0x1302549A: brw_compile_fs (brw_fs.cpp:6570) >> ==6164== by 0x130C4B07: blorp_compile_fs (blorp.c:194) >> ==6164== by 0x130D384B: blorp_params_get_clear_kernel >> (blorp_clear.c:79) >> ==6164== by 0x130D3C56: blorp_fast_clear (blorp_clear.c:332) >> ==6164== by 0x12EFA439: do_single_blorp_clear >> (brw_blorp.c:1261) >> ==6164== by 0x12EFC4AF: brw_blorp_clear_color >> (brw_blorp.c:1326) >> ==6164== by 0x12EFF72B: brw_clear (brw_clear.c:297) >> >> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com >> <mailto:tapani.pa...@intel.com>> >> --- >> src/intel/compiler/brw_fs.cpp | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/intel/compiler/brw_fs.cpp >> b/src/intel/compiler/brw_fs.cpp >> index 4616529abc..6b27c38be7 100644 >> --- a/src/intel/compiler/brw_fs.cpp >> +++ b/src/intel/compiler/brw_fs.cpp >> @@ -2092,7 +2092,7 @@ fs_visitor::assign_constant_locations() >> */ >> uint32_t *param = stage_prog_data->param; >> stage_prog_data->nr_params = num_push_constants; >> - stage_prog_data->param = ralloc_array(NULL, uint32_t, >> num_push_constants); >> + stage_prog_data->param = ralloc_array(mem_ctx, uint32_t, >> num_push_constants); >> >> >> Wow, I don't know how I didn't see this pass. The more correct >> answer is that blorp no longer uses push constants, so we can just >> delete the whole mess. I'll send a patch. >> >> >> Gah! Ignore me. This is, indeed, correct. >> >> if (num_pull_constants > 0) { >> stage_prog_data->nr_pull_params = num_pull_constants; >> stage_prog_data->pull_param = ralloc_array(NULL, uint32_t, >> >> >> We should be doing it here as well. >> >> > ok, did not catch that as the use case I was running did not use pull > constants, I can send a separate fix for that one. As I said in the commit message of the patch I sent, it *almost* doesn't matter in practice. It only matters if shader compilation somehow fails which is not a common case.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev