<div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 21, 2019 at 5:37 AM Erik Faye-Lund <<a href="mailto:erik.faye-lund@collabora.com" target="_blank">erik.faye-lund@collabora.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, 2019-10-18 at 18:23 -0400, Marek Olšák wrote:<br>
> Oh BTW, drivers that want to implement pipe_screen::finalize_nir to<br>
> improve cached shader compilation can't use any of those lowering<br>
> passes, because then things blow up.<br>
<br>
Two questions:<br>
<br>
1. What is pipe_screen::finalize_nir? I can't see that in master... Is<br>
it something that's currently cooking in a branch / MR?<br></blockquote><div><br></div><div>Yes, there is a merge request for it:</div><div><a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2181" target="_blank">https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2181</a></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
2. How will these things blow up?<br></blockquote><div><br></div><div>Incorrect rendering or GPU hangs.<br></div><div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> The only _optional_ lowering you can do in st/mesa is clamp_color and<br>
> force_persample_in_shader, which covers iris at least (radeonsi<br>
> doesn't need any). If you need more, you either need to invest a<br>
> significant amount of time to make driver-specific NIR work with the<br>
> lowering passes, or give up on good shader cache efficiency.<br>
<br>
I would love to hear something about why this is the case...<br></blockquote><div><br></div><div>finalize_nir creates a driver-specific NIR to some degree. Feature-lowering passes run after that, so they have to deal with NIR that they are not expecting, and it can be different for each driver.</div><div><br></div><div>I don't believe in lowering at the NIR level, because you don't get anything by optimizing e.g. clip plane code when your shaders have 1000 instructions and branches that you don't want to recompile from scratch. Lowering in st/mesa is for drivers that don't have the manpower to have good performance anyway, so it doesn't matter, and finalize_nir is not for them anyway (because they don't wanna make or can't afford to make the effort to make it work for them).<br></div><div><br></div><div>Marek<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> On Fri, Oct 18, 2019 at 4:17 PM Marek Olšák <<a href="mailto:maraeo@gmail.com" target="_blank">maraeo@gmail.com</a>> wrote:<br>
> > Note that none of the lowering passes work if I enable them on<br>
> > radeonsi. So if you think about enabling them for your driver, I<br>
> > guess you have some work to do in the driver as well.<br>
> > <br>
> > On the other hand, having multiple shader variants in st/mesa is a<br>
> > performance disadvantage. The lowering should be done at the<br>
> > machine-level assembly code, so that you don't have to completely<br>
> > recompile from a higher-level IR.<br>
> > <br>
> > Marek<br>
> > <br>
> > On Fri, Oct 18, 2019 at 4:08 PM Marek Olšák <<a href="mailto:maraeo@gmail.com" target="_blank">maraeo@gmail.com</a>><br>
> > wrote:<br>
> > > On Fri, Oct 18, 2019 at 9:07 AM Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a><br>
> > > > wrote:<br>
> > > > On Fri, Oct 18, 2019 at 9:04 AM Erik Faye-Lund<br>
> > > > <<a href="mailto:erik.faye-lund@collabora.com" target="_blank">erik.faye-lund@collabora.com</a>> wrote:<br>
> > > > ><br>
> > > > > On Fri, 2019-10-18 at 08:57 -0400, Ilia Mirkin wrote:<br>
> > > > > > On Fri, Oct 18, 2019 at 8:51 AM Erik Faye-Lund<br>
> > > > > > <<a href="mailto:erik.faye-lund@collabora.com" target="_blank">erik.faye-lund@collabora.com</a>> wrote:<br>
> > > > > > > On Thu, 2019-10-17 at 22:24 -0400, Ilia Mirkin wrote:<br>
> > > > > > > > In the meanwhile (unless you plan on taking up Jason's<br>
> > > > > > > > suggestion),<br>
> > > > > > > > might I recommend some assert's for the unhandled cases<br>
> > > > so that<br>
> > > > > > > > there<br>
> > > > > > > > are no surprises?<br>
> > > > > > ><br>
> > > > > > > Good idea. I sent a MR for it here:<br>
> > > > > > ><br>
> > > > > > > <br>
> > > > <a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2380" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2380</a><br>
> > > > > ><br>
> > > > > > Not sure that approach works, esp with SSO.<br>
> > > > ><br>
> > > > > If so, wouldn't that already be a problem with the existing<br>
> > > > > lower_depth_clamp-stuff, then? I mean, I just lifted the<br>
> > > > logic from<br>
> > > > > there...<br>
> > > > <br>
> > > > Yeah, that looks bogus. I'm moderately sure that checking<br>
> > > > "st->vp/gp/tep" at LinkShader time is wrong. Marek can probably<br>
> > > > confirm.<br>
> > > <br>
> > > Don't read any context states at link time. It's wrong.<br>
> > > <br>
> > > Marek<br>
> <br>
> _______________________________________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
<br>
</blockquote></div></div></div>