On 10/11/18 3:58 am, Jason Ekstrand wrote:
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.
For what its worth I like version B better also.
--Jason
On Thu, Nov 8, 2018 at 9:45 PM Jason Ekstrand <ja...@jlekstrand.net
<mailto: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
<mailto:ian.d.roman...@intel.com>>
Cc: Connor Abbott <cwabbo...@gmail.com <mailto: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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
