<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW - Implement a NIR -> vec4 pass" href="https://bugs.freedesktop.org/show_bug.cgi?id=89580#c6">Comment # 6</a> on <a class="bz_bug_link bz_status_NEW " title="NEW - Implement a NIR -> vec4 pass" href="https://bugs.freedesktop.org/show_bug.cgi?id=89580">bug 89580</a> from <a class="email" href="mailto:elima@igalia.com" title="Eduardo Lima Mitev <elima@igalia.com>"> Eduardo Lima Mitev</a> <pre>(In reply to Jason Ekstrand from <a href="show_bug.cgi?id=89580#c5">comment #5</a>) > (In reply to Eduardo Lima Mitev from <a href="show_bug.cgi?id=89580#c4">comment #4</a>) > > > > - load_const: For this, we did the same as fs_nir, allocating a new register > > and loading the constant on-demand, as part of get_nir_src(). We then > > realized that this is not optimal for us, since we can emit vector > > instructions and save a few movs. So we are planning to update the current > > implementation to have a map with a register for each SSA variable, and pick > > from that table when emitting. Thoughts on that? > > The reason for re-allocating and re-eimitting every time is that the > constant can be anywhere in the program (such as at the top) and we want the > copy-propagation pass to be able to propagate them. Having them in > different control flow can cause problems for this. > Hmm, yes we will have to take that into account. Pre-allocating registers for every ssa variable will have to be done carefully (i.e, in a per-implementation basis) to avoid unnecessary register spilling. I will give it some thought and ask you before implementing it. For now, current (unoptimized) solution is enough to move on. > I do have a couple of comments: > > 1) There are two places so far, brw_type_for_nir_type and > brw_conditional_for_nir_comparison, where we would like to put things in a > common place. Recently, I added a brw_nir.h file and I believe Ken added a > brw_nir.c file which would be a perfect place to put those. In fact, we > could probably simplify some of the current FS NIR code with your > conditionals function. > Yes, it is a good idea and will leave our code less cluttered. However, I will turn brw_nir.c to c++ to add these methods. Any caveat with that? > 2) We probably want to have emit_intrinsic be one big switch that does > everything like we do in the FS. I know that one big switch can be annoying > at times, but so is having a pile of little 3-line helpers that we have to > declare in the C++ class. Not a big deal either way. > Agree, will change that and take note. > Thank you for the extremely prompt (given that it was the middle of the > night there) reply! Keep up the good work! Well, it was totally worth the effort getting some code ready for you to look at. Thanks a lot for the early review!</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the QA Contact for the bug.</li> </ul> </body> </html>