On Mon, Feb 23, 2015 at 10:02:26AM -0800, Matt Turner wrote: > On Sun, Feb 22, 2015 at 3:06 PM, Ben Widawsky <b...@bwidawsk.net> wrote: > > On Sun, Feb 08, 2015 at 02:48:02PM -0800, Matt Turner wrote: > >> On Sun, Feb 8, 2015 at 1:59 PM, Ben Widawsky > >> <benjamin.widaw...@intel.com> wrote: > >> > + /* The LOAD_PAYLOAD helper seems like the obvious choice here. > >> > However, it > >> > + * requires a lot of information about the sources to appropriately > >> > figure > >> > + * out the number of registers needed to be used. Given this stage > >> > in our > >> > + * optimization, we may not have the appropriate GRFs required by > >> > + * LOAD_PAYLOAD at this point (copy propogation). Therefore, we need > >> > to > >> > >> typo: propagation > >> > >> I'm not sure what "w e may not have the appropriate GRFs ..." means? > > > > Here is the relevant part of the original IRC conversation: > > jekstrand [08:52:30] No, the problem is uniforms and immediates. > > jekstrand [08:52:58] They can't go in a LOAD_PAYLOAD directly because > > we don't know how many destination registers they take up. > > jekstrand [08:54:16] for LOAD_PAYLOAD to work, we need more > > information than a regular instruction. We need to know how many > > destination registers a given source takes up, we need to know whether it > > needs to use the second-half quarter control for the MOV that gets > > generated, etc. > > jekstrand [08:54:34] Using GRF sources more-or-less gives us this. > > Immediates don't. > > bwidawks [08:54:55] right - this is what confuses me though... the > > immediates seem to already be there. > > jekstrand [08:55:38] Right. The immediates can get there through > > copy-propagation and that's fine. However, they're not there when it's > > created. > > jekstrand [08:55:43] It's all a mess > > > > Do you have a preferred way to state this concisely? > > Heh, I'm not sure I understand LOAD_PAYLOAD anymore. The comment's > probably fine as-is. > > >> > @@ -3609,6 +3709,7 @@ fs_visitor::optimize() > >> > OPT(opt_peephole_predicated_break); > >> > OPT(opt_cmod_propagation); > >> > OPT(dead_code_eliminate); > >> > + OPT(opt_sampler_eot); > >> > >> Do you think we really need to do this in the optimization loop? > >> > >> I don't expect this to allow other optimization passes to make > >> additional progress, and we can obviously do it successfully only > >> once. I suspect we can do it after the optimization loop. > >> > > > > It's possible I didn't quite spot where you want me to put the > > optimization. I > > think that the way the code works right now, that will not work. The > > optimization is depending on DCE to kill off the only LOAD_PAYLOAD and it's > > corresponding MOVs. I agree that it is an optimization that can only occur > > once, > > and generally it doesn't belong in the progress loop though. > > Ah, sorry. I'd probably do it between the end of the optimization loop > and the call to lower_load_payload().
Right. I looked at this too and at least from the 5 second glance at the code, it seems this path can skip DCE. If you are certain that this cannot happen, I will gladly make the change. I defer to you regarding whether or not the optimizations can do more after this happens (ie. we definitely want to take it out of the loop). _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
