On Fri, May 27, 2016 at 1:16 PM, Jason Ekstrand <[email protected]> wrote:
> > > On Fri, May 27, 2016 at 1:10 PM, Francisco Jerez <[email protected]> > wrote: > >> Jason Ekstrand <[email protected]> writes: >> >> > On Thu, May 26, 2016 at 8:46 PM, Francisco Jerez <[email protected] >> > >> > wrote: >> > >> >> --- >> >> src/mesa/drivers/dri/i965/brw_fs.cpp | 26 ++++++++++++++++++++++++++ >> >> 1 file changed, 26 insertions(+) >> >> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp >> >> b/src/mesa/drivers/dri/i965/brw_fs.cpp >> >> index 1f3b23b..7002346 100644 >> >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp >> >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp >> >> @@ -6547,6 +6547,32 @@ brw_compile_cs(const struct brw_compiler >> *compiler, >> >> void *log_data, >> >> } >> >> } >> >> >> >> + fs_visitor v32(compiler, log_data, mem_ctx, key, &prog_data->base, >> >> + NULL, /* Never used in core profile */ >> >> + shader, 32, shader_time_index); >> >> + if (!fail_msg && v8.max_dispatch_width >= 32 && >> >> + simd_required == 32) { >> >> >> > >> > I don't see where simd_required is getting aligned up to a power-of-two >> so >> > how are we expecting to hit this? Also, I took a look at the SIMD16 >> case >> > above, and we're hand-rolling simd_required there (which we shouldn't >> be). >> > Here's what I would suggest: >> > >> >> Yeah, I had noticed that last night shortly after sending, the version >> of this patch I pushed in the branch shouldn't have this bug: >> >> >> https://cgit.freedesktop.org/~currojerez/mesa/commit/?h=i965-simd32-cs&id=3d7f22ffd567901c9561b93b0d6945a50a095997 >> >> > simd_required = DIV_ROUND_UP(...) >> > min_simd = 32 >> > >> > then, in each one we do >> > >> > if ((INTEL_DEBUG & DEBUG_NO16) && simd_required <= 16 && min_simd >= >> 16) { >> > if (min_simd == 8) >> > v16.import_uniforms(v8) >> > if (!v16.run_cs()) { >> > /* fail */ >> > } else { >> > /* success */ >> > min_simd = MIN2(min_simd, 16); >> > } >> > } >> > >> > with the obvious adjustments for 8 and 32. That way no8 and no16 both >> work >> > fine and we properly guarantee that we compile exactly one shader. >> > >> >> So if I'm understanding correctly you're asking me to rework the SIMD16 >> and SIMD8 back-end invocation so the no8 and no16 debugging options are >> taken into account for compute shaders? That would definitely make >> PATCH 23 unnecessary but will be somewhat more churn (not saying that it >> wouldn't make sense to rewrite brw_compile_cs() eventually). >> > > Well, we already take no16 into account and the uniform import code here > (and in the patch above) won't work if you use INTEL_DEBUG=no16 and need > more than 8 because it's using simd_required for both the minimum and > maximum. In other words, I think it's horribly broken and we should fix > it. If you don't want to fix it now for the sake of time, I understand, > but we should back-port the fix so that it's not broken in 12.0. > I looked a bit harder at this and it's more complex than I first thought. Other than the obvious simd_required fix, I'm not 100% sure what it should look like. Go ahead and just push the version in your branch. > --Jason > > >> > Seem reasonable? >> > >> > --Jason >> > >> > >> >> + /* Try a SIMD32 compile */ >> >> + if (simd_required <= 8) >> >> + v32.import_uniforms(&v8); >> >> + else if (simd_required <= 16) >> >> + v32.import_uniforms(&v16); >> >> + >> >> + if (!v32.run_cs()) { >> >> + compiler->shader_perf_log(log_data, >> >> + "SIMD32 shader failed to compile: >> %s", >> >> + v16.fail_msg); >> >> + if (!cfg) { >> >> + fail_msg = >> >> + "Couldn't generate SIMD32 program and not " >> >> + "enough threads for SIMD16"; >> >> + } >> >> + } else { >> >> + cfg = v32.cfg; >> >> + prog_data->simd_size = 32; >> >> + } >> >> + } >> >> + >> >> if (unlikely(cfg == NULL)) { >> >> assert(fail_msg); >> >> if (error_str) >> >> -- >> >> 2.7.3 >> >> >> >> _______________________________________________ >> >> mesa-dev mailing list >> >> [email protected] >> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> >> >> > >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
