<div dir="ltr"><div>Oh BTW, drivers that want to implement pipe_screen::finalize_nir to improve cached shader compilation can't use any of those lowering passes, because then things blow up. The only _optional_ lowering you can do in st/mesa is clamp_color and force_persample_in_shader, which covers iris at least (radeonsi doesn't need any). If you need more, you either need to invest a significant amount of time to make driver-specific NIR work with the lowering passes, or give up on good shader cache efficiency.<br></div><div><br></div><div>Marek<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">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></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>Note that none of the lowering passes work if I enable them on radeonsi. So if you think about enabling them for your driver, I guess you have some work to do in the driver as well.</div><div><br></div><div>On the other hand, having multiple shader variants in st/mesa is a performance disadvantage. The lowering should be done at the machine-level assembly code, so that you don't have to completely recompile from a higher-level IR.<br></div><div><br></div><div>Marek<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Oct 18, 2019 at 4:08 PM Marek Olšák <<a href="mailto:maraeo@gmail.com" target="_blank">maraeo@gmail.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"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">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>> 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, 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 so that<br>
> > > > there<br>
> > > > are no surprises?<br>
> > ><br>
> > > Good idea. I sent a MR for it here:<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 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></blockquote><div><br></div>Don't read any context states at link time. It's wrong.</div><div class="gmail_quote"><br></div><div class="gmail_quote">Marek<br></div></div>
</blockquote></div>
</blockquote></div>