In case anyone is wondering my opinion based on my experience writing both patch series, I think it's decidedly in the version B camp. The code changes required to nir_algebraic and nir_search are almost identical between the two besides the whole virtual opcode thing and the changes to front-end NIR producers are basically the same as well due to having to put 32 suffixes on some things. However, the changes to other parts of NIR aren't required and I feel they're very dirty and special-casey in version A.
--Jason On Thu, Nov 8, 2018 at 9:45 PM Jason Ekstrand <ja...@jlekstrand.net> wrote: > Welcome to this choose-your-own-adventure patch series! The first six > patches are good-to-go regardless of choices made later. However, starting > with patch 7, we have a choice to make. > > Version A of this series reworks NIR conversion opcodes to not have sizes > and instead simply accept any source and destination size regardless of > whether or not they match. This is something that Connor and I talked > about some time ago when we were first adding bit sizes. With most > opcodes, if they have any variable-size sources or a variable size > destination, all variable sized things must match. This series changes the > rules so that, for conversion ops, they have a variable size source and > destination which DO NOT have to match. The upside to this is that we get > to throw away piles of opcodes. The downside is that it makes a number of > things: validation, nir_search, constant folding, etc. more complicated > because they now have to deal with a new set of rules. > > Version B of the series, reworks boolean conversions to be like the current > conversions and also have bit sizes. Unfortunately, this makes hash of > opt_algebraic because, thanks to some applications love of re-inventing > Boolean operations, we have piles of optimizations that contain b2f, f2b, > b2i and i2b. To deal with that problem (and make opt_algebraic easier to > use in general), version B adds virtual unsized conversion opcodes to > nir_search so that you can make an expression that matches any i2f opcode > regardless of size. From the perspective of opt_algebraic, then, the two > series are nearly identical. The downside of this approach is that the > nir_search changes are kind-of hacky and we still have piles of conversion > opcodes. The upside, however, is that we don't have the extra mental > overhead in constant folding, validation, and elsewhere of having this > opcode special-case. > > What is the fate of NIR? That's up to the people to decide! > > Cc: Ian Romanick <ian.d.roman...@intel.com> > Cc: Connor Abbott <cwabbo...@gmail.com> > > Jason Ekstrand COMMON (6): > nir/opcodes: Pull in the type helpers from constant_expressions > nir/opcodes: Rename tbool to tbool32 > nir/algebraic: Improve a couple error messages > nir/algebraic: Set variable classes to the explicit bit size > nir/algebraic: Clean up some __str__ cruft > nir/algebraic: Improve error messages for replace expressions > > Jason Ekstrand VERSION A (9): > nir: Add a concept of an unsized conversion opcode > nir/builder: Create sized helpers for unsized conversion opcodes > nir/constant_expressions: Handle unsized conversion ops > nir/algebraic: Add support for unsized conversion opcodes > nir: Switch conversions to unsized opcodes > FIXUP: Fix NIR producers and consumers to use unsized conversions > FIXUP: nir/opt_algebraic: Drop bit-size suffixes from conversions > nir/algebraic: Add 32-bit specifiers to a bunch of booleans > nir: Make boolean conversions unsized like other types > > Jason Ekstrand VERSION B (8): > nir/algebraic: Refactor codegen a bit > nir/algebraic: Add support for unsized conversion opcodes > nir/opt_algebraic: Simplify an optimization using the new search ops > nir/opt_algebraic: Drop bit-size suffixes from conversions > nir/opt_algebraic: Add 32-bit specifiers to a bunch of booleans > nir: Make boolean conversions sized just like the others > FIXUP: nir/opt_algebraic: Add suffixes to some x2b opcodes > FIXUP: Fix NIR producers and consumers to use unsized conversions > > src/amd/common/ac_nir_to_llvm.c | 32 +-- > src/compiler/glsl/glsl_to_nir.cpp | 2 +- > src/compiler/nir/nir.c | 67 ++++++ > src/compiler/nir/nir.h | 7 + > src/compiler/nir/nir_algebraic.py | 174 +++++++++++---- > src/compiler/nir/nir_builder.h | 30 ++- > src/compiler/nir/nir_builder_opcodes_h.py | 16 +- > src/compiler/nir/nir_constant_expressions.h | 3 +- > src/compiler/nir/nir_constant_expressions.py | 59 +++--- > src/compiler/nir/nir_loop_analyze.c | 7 +- > src/compiler/nir/nir_lower_idiv.c | 2 +- > src/compiler/nir/nir_lower_int64.c | 2 +- > src/compiler/nir/nir_opcodes.py | 152 +++++++------ > src/compiler/nir/nir_opcodes_c.py | 89 +------- > src/compiler/nir/nir_opt_algebraic.py | 199 +++++++++--------- > src/compiler/nir/nir_opt_constant_folding.c | 5 +- > src/compiler/nir/nir_search.c | 19 +- > src/compiler/nir/nir_validate.c | 4 + > src/compiler/spirv/spirv_to_nir.c | 3 +- > src/compiler/spirv/vtn_glsl450.c | 4 +- > src/gallium/auxiliary/nir/tgsi_to_nir.c | 8 +- > .../drivers/freedreno/ir3/ir3_compiler_nir.c | 148 ++++++------- > src/gallium/drivers/vc4/vc4_program.c | 8 +- > src/intel/compiler/brw_fs_nir.cpp | 78 +++---- > src/intel/compiler/brw_vec4_nir.cpp | 39 ++-- > src/mesa/program/prog_to_nir.c | 4 +- > 26 files changed, 626 insertions(+), 535 deletions(-) > > -- > 2.19.1 > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
