[Mesa-dev] [PATCH 00/15] nir: Rework boolean (and maybe all?) conversions

Fri Nov 9 16:58:50 UTC 2018

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 <jason at 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.romanick at intel.com>
> Cc: Connor Abbott <cwabbott0 at 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181109/a59a4e8e/attachment.html>