<html dir="ltr"><head></head><body style="text-align:left; direction:ltr;"><div>I just noticed I never replied to this:</div><div><br></div><div>On Tue, 2018-10-30 at 10:18 -0500, Jason Ekstrand wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Oct 30, 2018 at 2:50 AM Iago Toral <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote:<br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">Some quick comments on this after experimenting with it:<br>
<br>
Intel hardware produces booleans of the same bit-size as the operations<br>
that generates them so I am working on expanding this to add another<br>
lowering pass that can lower them to smaller bit-sizes.<br>
<br>
I have a work-in-progress prototype of this and it is significantly<br>
easier, at least for us, to do this as a lowering pass in NIR SSA form<br>
than in the backend. Specifically, doing it in the backend means that<br>
we have to deal with propagating the bit-sizes of the booleans across<br>
operation chains and across CFG blocks, which isn't fun.<br>
<br>
There are still a few things that I am figuring out:<br>
<br>
Right now I lower instructions as I find them, deciding about the bit-<br>
size of the destination based on the bit-size of the sources. This<br>
usually works fine because by the time we reach an instruction that<br>
reads boolean sources we have already lowered the instructions that<br>
produced them. However, this also means that we need to make decisions<br>
about the bit-sizes of constant booleans before we know which bit-size<br>
is required for them, it could be that we need them as 32-bit booleans<br>
only, 16-bit booleans only, or maybe both. Right now I am assuming that<br>
constants are 32-bit always, which means that at the moment I have to<br>
inject a MOV when they are used in 16-bit instructions. We can probably<br>
do better, at least in some cases, if we inspect all uses of the<br>
constants before we lower them.<br></blockquote><div><br></div><div>If we did a step of constant folding, wouldn't the constant booleans get nicely folded to the right bit-size?</div></div></div></blockquote><div><br></div><div>The lowering pass runs late, so we have already run constant folding by the time we get here. Anyway, I have double checked and the load_const instructions seem to come with bit-size > 1 in all the cases I have available from CTS, so maybe this just works already and we never hit this. With that said, I am not 100% sure if there are any cases where we could still end up having to lower a 1-bit bool load const instruction and figuring our a bit-size for it, but at the very least this looks like it would not be a common case anyway, so maybe we can deal with that whenwe find an actual case.</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
Another thing we need to deal with is canonicalization. Some<br>
instructions may have boolean sources of different native bit-sizes and<br>
it looks like we need to choose a canonical bit-size for them in this<br>
case (at least in my experiments this seems to be required for Intel).<br>
For now I am just taking the bit-size of the first source as the<br>
canonical form, but we could try to be smarter about this. In the case<br>
of Intel I think this isn't too bad, because constant operands are<br>
usually second, so for instructions that use a boolean immediate, if we<br>
choose the first source as canonical, we can then adjust the imediate<br>
source without having to emit additional instructions. Unfortunately,<br>
the 16-bit integer tests in CTS are not good to test set for<br>
canonicalization, since they focus very specifically in 16-bit paths<br>
and I don't think we have many cases (if any) of boolean instructions<br>
mixing booleans of different bit-sizzes, so we will probably have to<br>
try and write our more tests to exercise these cases properly. Also, it<br>
might be that some hardware that supports multiple bit-sized booleans<br>
doesn't require canonicalization, in which case we might want to do<br>
this configurable.<br></blockquote><div><br></div><div>Assuming a back-end that can properly handle 8 and 16-bit booleans, I suspect "always go down" would be a reasonable for two reasons:</div><div><br></div><div> 1) It always uses no more bits than the biggest so it helps with register pressure</div><div> 2) We can do the conversion "for free" either with a mixed-type operation (or of D and W) or, if that doesn't work, by using a register region to just reinterpret the D as a scattered out W.</div><div><br></div><div>For that matter, there's a part of me that's tempted to always spill to 8-bit for everything other than the comparison operation itself. Again, this is all dependent on having a competent back-end compiler which we currently don't have. :-(<br></div></div></div></blockquote><div><br></div><div>Right, doing 8-bit is probably not a good idea for now. Byte instructions have this annoying restriction by which destinations need to have a stride of 2 which I think doesn't play well with the optimizer in general right now, but beyond that, the hardware doesn't support byte immediates, which means that we lose algebraic and cmod optimizations that rely on the is_zero(), is_one() and is_negative_one() immediate helpers to work. This affects, for example, SEL instructions, of which we have plenty, where we need emit a cmp with 0 to right before to prepare the flag register. With the lowering pass I have right now I see clearly better instruction counts for 16-bit code but I still get worse counts for 8-bit booleans (compared to always using 32-bit booleans) because of these reasons. I think that not having byte immediates might be bad enough that we might want to avoid emitting 8-bit booleans completely in the end. I still want to test a few tings related to this though, such as trying to use 16-bit immediates with 8-bit instructions and see if that works...</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
Another thing I noticed, is that with phi sources the canonicalization<br>
aspect is a bit trickier, since we can't just insert conversions before<br>
the phi instructions (nir_validate complains that this is not allowed),<br>
so instead we need to get to the instruction that generated the ssa<br>
source and inject the conversion after it.<br></blockquote><div><br></div><div>There's a nifty nir_before_src cursor builder function that correctly handles phis.<br></div></div></div></blockquote><div><br></div><div>Yeah, that's useful, thanks for pointing it out! One question about this, the helper receives an 'is_if_condition' parameter which I guess should be false for phi sources, is that correct?</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">
Last thing I noticed is that our backend optimizer is much better at<br>
optimizing 32-bit booleans that it is at optimizing 16-bit, so there<br>
are some cases where we end up producing worse code. Obviously this is<br>
not a problem of the NIR lowering pass though, it just means that we<br>
need to do better at optimizing 16-bit code in the backend.<br>
<br>
So all in all, I am quite happy with this. It is certainly easier than<br>
lowering in the backend and it gives us a more reasonable framework to<br>
work with. We still need to do some extra work if we want to do the<br>
best lowering for things like boolean constants or canonicalization,<br>
but since neither SPIR-V nor GLSL define multiple bit-sizes for<br>
booleans that would always have been something that drivers might need<br>
to address.<br>
<br>
Iago<br>
<br>
<br>
On Mon, 2018-10-22 at 17:13 -0500, Jason Ekstrand wrote:<br>
> This is something that Connor and I have talked about quite a bit<br>
> over the<br>
> last couple of months. The core idea is to replace NIR's current 32-<br>
> bit<br>
> 0/-1 D3D10-style booleans with a 1-bit data type. All in all, I<br>
> think it<br>
> worked out pretty well though I really don't like the proliferation<br>
> of<br>
> 32-bit comparison opcodes we now have kicking around for i965.<br>
> <br>
> Why? No hardware really has a 1-bit type, right? Well, sort<br>
> of... AMD<br>
> actually uses 64-bit scalars for booleans with one bit per<br>
> invocation.<br>
> However, most hardware such as Intel uses some other larger value for<br>
> booleans. The real benefit of 1-bit booleans and requiring a<br>
> lowering pass<br>
> is that you can do somewhat custom lowering (like AMD wants) and your<br>
> lowering pass can always tell in an instant if a value is a boolean<br>
> based<br>
> on the bit size. As can be seen in the last patch, this makes it<br>
> really<br>
> easy to implement a bool -> float lowering pass for hardware that<br>
> doesn't<br>
> have real integers where NIR's current booleans are actually rather<br>
> painful.<br>
> <br>
> On Intel, the situation is a bit more complicated. It's tempting to<br>
> say<br>
> that we have 32-bit D3D10 booleans. However, they're not really<br>
> D3D10<br>
> booleans on gen4-5 because the top 31 bits are undefined garbage and,<br>
> while<br>
> iand, ior, ixor, and inot operations work, you have to iand with 1 at<br>
> the<br>
> last minute to clear off all that garbage. Also, on all generations,<br>
> a<br>
> comparison of two N-bit values results in an N-bit boolean, not a 32-<br>
> bit<br>
> bool. This has caused the Igalia folks no end of trouble as they've<br>
> been<br>
> working on native 8 and 16-bit support. If, instead, we have a 1-bit <br>
> bool<br>
> with a lowering pass and we can lower to whatever we want, then we<br>
> could<br>
> lower to a set of comparison opcodes that return the same bit-size as<br>
> they<br>
> compare and it would match GEN hardware much better.<br>
> <br>
> But what about performance? Aren't there all sorts of neat tricks we<br>
> can<br>
> do with D3D10 booleans like b & 1.0f for b2f? As it turns out, not<br>
> really;<br>
> that's about the only one. There is some small advantage when<br>
> optimizing<br>
> shaders that come from D3D if your native representation of booleans<br>
> matches that of D3D. However, penultimate patch in this series adds<br>
> a few<br>
> small optimizations that get us to actually better than we were<br>
> before.<br>
> With the entire series, shader-db on Kaby Lak looks like this:<br>
> <br>
> total instructions in shared programs: 15084098 -> 14988578 (-<br>
> 0.63%)<br>
> instructions in affected programs: 1321114 -> 1225594 (-7.23%)<br>
> helped: 2340<br>
> HURT: 23<br>
> <br>
> total cycles in shared programs: 369790134 -> 359798399 (-2.70%)<br>
> cycles in affected programs: 134085452 -> 124093717 (-7.45%)<br>
> helped: 2149<br>
> HURT: 720<br>
> <br>
> total loops in shared programs: 4393 -> 4393 (0.00%)<br>
> loops in affected programs: 0 -> 0<br>
> helped: 0<br>
> HURT: 0<br>
> <br>
> total spills in shared programs: 10158 -> 10051 (-1.05%)<br>
> spills in affected programs: 1429 -> 1322 (-7.49%)<br>
> helped: 8<br>
> HURT: 15<br>
> <br>
> total fills in shared programs: 22105 -> 21720 (-1.74%)<br>
> fills in affected programs: 2853 -> 2468 (-13.49%)<br>
> helped: 12<br>
> HURT: 15<br>
> <br>
> How about ease of use? Are they a pain to deal with? Yes, adding<br>
> support<br>
> for 1-bit types was a bit awkward in a few places but most of it was<br>
> dealing with all the places where we have 32-bit booleans baked into<br>
> assumptions. Getting rid of that baking in solves the problem and<br>
> also<br>
> makes the whole IR more future-proof.<br>
> <br>
> All in all, I'd say I'm pretty happy with it. However, I'd like<br>
> other<br>
> people (particularly the AMD folks) to play with it a bit and verify<br>
> that<br>
> it solves their problems as well. Also, I added a lowering pass and<br>
> tried<br>
> to turn it on in everyone's driver but may not have put it in the<br>
> right<br>
> spot. Please double-check my work. For those wishing to take a<br>
> look, you<br>
> can also find the entire series on my gitlab here:<br>
> <br>
> <br>
<a href="https://gitlab.freedesktop.org/jekstrand/mesa/commits/review/nir-1-bit-bool" rel="noreferrer" target="_blank">https://gitlab.freedesktop.org/jekstrand/mesa/commits/review/nir-1-bit-bool</a><br>
> <br>
> Please review!<br>
> <br>
> --Jason<br>
> <br>
> Cc: Connor Abbott <<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>><br>
> Cc: Timothy Arceri <<a href="mailto:tarceri@itsqueeze.com" target="_blank">tarceri@itsqueeze.com</a>><br>
> Cc: Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank">eric@anholt.net</a>><br>
> Cc: Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>><br>
> Cc: Karol Herbst <<a href="mailto:karolherbst@gmail.com" target="_blank">karolherbst@gmail.com</a>><br>
> Cc: Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl" target="_blank">bas@basnieuwenhuizen.nl</a>><br>
> Cc: Alyssa Rosenzweig <<a href="mailto:alyssa@rosenzweig.io" target="_blank">alyssa@rosenzweig.io</a>><br>
> <br>
> <br>
> Jason Ekstrand (31):<br>
> nir/validate: Print when the validation failed<br>
> nir/constant_folding: Add an unreachable to a switch<br>
> nir/constant_folding: Use nir_src_as_bool for discard_if<br>
> nir/builder: Add a nir_imm_true/false helpers<br>
> nir/builder: Handle 16-bit floats in nir_imm_floatN_t<br>
> nir/search: Use nir_builder<br>
> nir/opt_if: Rework condition propagation<br>
> nir/system_values: Use the bit size from the load_deref<br>
> glsl/nir: Use i2b instead of ine for fixing UBO/SSBO booleans<br>
> nir/prog: Use nir_bany in kill handling<br>
> spirv: Use the right bit-size for spec constant ops<br>
> spirv: Initialize subgroup destinations with the destination type<br>
> intel/nir: Use the OPT macro for more passes<br>
> nir/algebraic: Disable b2f lowering and two optimizations<br>
> nir: Rename boolean-related opcodes to include 32 in the name<br>
> FIXUP: nir/builder: Generate 32-bit bool opcodes transparently<br>
> FIXUP: nir/algebraic: Remap boolean opcodes to the 32-bit variant<br>
> FIXUP: Use 32-bit opcodes in the NIR producers and optimizations<br>
> FIXUP: Use 32-bit opcodes in the NIR back-ends<br>
> nir/constant_expressions: Rework boolean handling<br>
> nir: Add support for 1-bit data types<br>
> nir: Add 1-bit boolean opcodes<br>
> nir: Add a bool to int32 lowering pass<br>
> glsl,spirv: Generate 1-bit booleans<br>
> FIXUP: Revert "Use 32-bit opcodes in the NIR producers and<br>
> optimizations"<br>
> FIXUP: Revert "nir/builder: Generate 32-bit bool opcodes<br>
> transparently"<br>
> FIXUP: nir/builder: Generate 1-bit booleans in nir_build_imm_bool<br>
> nir/algebraic: Optimize 1-bit booleans<br>
> nir/algebraic: Add back lower_b2f and a b2f optimization<br>
> nir/algebraic: Add some optimizations for D3D-style booleans<br>
> nir: Add a bool to float32 lowering pass<br>
> <br>
> src/amd/common/ac_nir_to_llvm.c | 30 +--<br>
> src/amd/vulkan/radv_shader.c | 6 +-<br>
> src/broadcom/compiler/nir_to_vir.c | 44 ++---<br>
> src/broadcom/compiler/vir.c | 2 +<br>
> src/compiler/Makefile.sources | 2 +<br>
> src/compiler/glsl/glsl_to_nir.cpp | 28 +--<br>
> src/compiler/nir/meson.build | 2 +<br>
> src/compiler/nir/nir.c | 15 +-<br>
> src/compiler/nir/nir.h | 36 ++--<br>
> src/compiler/nir/nir_algebraic.py | 22 ++-<br>
> src/compiler/nir/nir_builder.h | 51 ++++-<br>
> src/compiler/nir/nir_constant_expressions.py | 53 ++---<br>
> src/compiler/nir/nir_instr_set.c | 23 ++-<br>
> src/compiler/nir/nir_lower_alu_to_scalar.c | 4 +<br>
> src/compiler/nir/nir_lower_bool_to_float.c | 181<br>
> ++++++++++++++++++<br>
> src/compiler/nir/nir_lower_bool_to_int32.c | 162 ++++++++++++++++<br>
> src/compiler/nir/nir_lower_int64.c | 2 +-<br>
> .../nir/nir_lower_load_const_to_scalar.c | 3 +<br>
> src/compiler/nir/nir_lower_returns.c | 4 +-<br>
> src/compiler/nir/nir_lower_subgroups.c | 2 +-<br>
> src/compiler/nir/nir_lower_system_values.c | 1 +<br>
> src/compiler/nir/nir_opcodes.py | 31 ++-<br>
> src/compiler/nir/nir_opt_algebraic.py | 19 +-<br>
> src/compiler/nir/nir_opt_constant_folding.c | 18 +-<br>
> src/compiler/nir/nir_opt_if.c | 91 +++------<br>
> src/compiler/nir/nir_opt_intrinsics.c | 2 +-<br>
> src/compiler/nir/nir_opt_large_constants.c | 5 +<br>
> src/compiler/nir/nir_print.c | 3 +<br>
> src/compiler/nir/nir_search.c | 112 +++--------<br>
> src/compiler/nir/nir_search.h | 9 +-<br>
> src/compiler/nir/nir_validate.c | 16 +-<br>
> src/compiler/nir/tests/vars_tests.cpp | 38 ++--<br>
> src/compiler/nir_types.cpp | 2 +-<br>
> src/compiler/nir_types.h | 4 +-<br>
> src/compiler/spirv/spirv_to_nir.c | 23 ++-<br>
> src/compiler/spirv/vtn_alu.c | 4 +-<br>
> src/compiler/spirv/vtn_cfg.c | 10 +-<br>
> src/compiler/spirv/vtn_subgroup.c | 12 +-<br>
> src/gallium/auxiliary/nir/tgsi_to_nir.c | 20 +-<br>
> .../drivers/freedreno/ir3/ir3_compiler_nir.c | 31 +--<br>
> src/gallium/drivers/radeonsi/si_shader_nir.c | 2 +<br>
> src/gallium/drivers/vc4/vc4_program.c | 50 ++---<br>
> src/intel/compiler/brw_fs_nir.cpp | 80 ++++----<br>
> src/intel/compiler/brw_nir.c | 16 +-<br>
> .../brw_nir_analyze_boolean_resolves.c | 24 +--<br>
> .../compiler/brw_nir_lower_image_load_store.c | 2 +-<br>
> src/intel/compiler/brw_vec4_nir.cpp | 122 ++++++------<br>
> src/intel/vulkan/anv_pipeline.c | 2 +-<br>
> src/mesa/drivers/dri/i965/brw_program.c | 4 +-<br>
> src/mesa/main/glspirv.c | 2 +-<br>
> src/mesa/program/prog_to_nir.c | 2 +-<br>
> src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-<br>
> 52 files changed, 934 insertions(+), 497 deletions(-)<br>
> create mode 100644 src/compiler/nir/nir_lower_bool_to_float.c<br>
> create mode 100644 src/compiler/nir/nir_lower_bool_to_int32.c<br>
> <br>
<br>
</blockquote></div></div>
</blockquote></body></html>