[Mesa-dev] [PATCH 00/21] NIR control flow modification
kenneth at whitecape.org
Thu Aug 6 17:29:07 PDT 2015
On Tuesday, July 21, 2015 07:54:14 PM Connor Abbott wrote:
> Back when I was getting NIR up and running, I created a bunch of
> functions that dealt with modifying control flow; they allowed you to
> remove control flow nodes (if's, loops, and basic blocks) as well as
> insert newly-created control flow nodes at certain points. The insertion
> API's worked well enough for the usecase we had, which was building up
> NIR while translating from something else, and the removal API found
> some use in e.g. the select peephole. But there are various things we
> want to do, some of which are becoming more and more urgent, which
> involve doing more invasive modifications to the IR. To be able to do
> these things, this series adds an API which is a little lower-level than
> the existing one, but a lot less dangerous than manipulating things
> directly. It uses most of the same internal machinery as the existing
> functions, but I needed to rework it extensively to fix issues that were
> usually obscure edges in the old code that become more important in the
> new code. For more detail on the API itself and how to use it, see patch
> 20. In addition, there are a few peripheral things that I did which are
> related to the core objective of the series:
> - The control flow code was entangled with the rest of the code in
> nir.c, and it was about to get larger, so I pulled it out into its own
> file (nir_control_flow.c) and gave it its own header
> - The new API includes an extract function (for details see patch 20)
> which needs to take begin and end points. There were going to be many
> variants necessary for completeness, and even more for convenience, so
> I decided to do something about it by creating a "cursor" structure. A
> cursor is a small structure (8 bytes on a 32-bit system, 16 bytes on a
> 64-bit system) that represents some point where something can be
> inserted (before or after an instruction or before or after a block).
> There are helpers for constructing cursors in all the relevant places,
> and convenience helpers for creating cursors before/after a control
> flow node and before/after a control flow list. That way, the need for
> trivial nir_insert_thing_before/after_other_thing wrappers is gone,
> and it's easy to add new convenience helpers for cursors which will
> work with every API that uses a cursor. We probably want to use
> cursors to simplify instruction insertion/deletion and the builder,
> but I deferred that till later.
> The series was tested with an updated version of my dead control flow
> elimination series, which I'm going to post shortly.
> The structure of the series is as follows:
> patch 01: adds a lot more checking of block predecessors and successors
> in the validator, which is useful for checking the results of control
> flow modification.
> patch 02-03: prepares for splitting off the control flow code by getting
> rid of as many dependencies as possible.
> patch 04: moves the control flow code off to its own file.
> patch 05-15: fix many different problems with the current control flow
> code which will become more important when the new API is added.
> patch 16-19: introduce the cursor helper, and make the old control flow
> insertion API use it.
> patch 20: actually adds the new API.
> patch 21: removes the old, buggy implementation of nir_cf_node_remove()
> and makes it a small wrapper around the new API instead.
> The series can also be found at:
> git://people.freedesktop.org/~cwabbott0/mesa nir-control-flow-mod
> Connor Abbott (21):
> nir/validate: check successors/predecessors more carefully
> nir: inline block_add_pred() a few places
> nir: make cleanup_cf_node() not use remove_defs_uses()
> nir: move control flow modification to its own file
> nir/cf: add insert_phi_undef() helper
> nir: add nir_foreach_phi_src_safe()
> nir/cf: add remove_phi_src() helper
> nir/cf: split up and improve nir_handle_remove_jumps()
> nir/cf: handle phi nodes better in split_block_beginning()
> nir/cf: add block_ends_in_jump()
> nir/cf: handle jumps in split_block_end()
> nir/cf: handle jumps better in stitch_blocks()
> nir/cf: remove uses of SSA definitions that are being deleted
> nir/cf: clean up jumps when cleaning up CF nodes
> nir/cf: fix link_blocks() when there are no successors
> nir/cf: add a cursor structure
> nir/cf: add split_block_before_instr()
> nir/cf: add split_block_cursor()
> nir/cf: use a cursor for inserting control flow
> nir/cf: add new control modification API's
> nir/cf: reimplement nir_cf_node_remove() using the new API
> src/gallium/auxiliary/nir/tgsi_to_nir.c | 1 +
> src/glsl/Makefile.sources | 3 +
> src/glsl/nir/glsl_to_nir.cpp | 1 +
> src/glsl/nir/nir.c | 680 +---------------------------
> src/glsl/nir/nir.h | 17 +-
> src/glsl/nir/nir_control_flow.c | 773 ++++++++++++++++++++++++++++++++
> src/glsl/nir/nir_control_flow.h | 252 +++++++++++
> src/glsl/nir/nir_control_flow_private.h | 38 ++
> src/glsl/nir/nir_opt_peephole_select.c | 1 +
> src/glsl/nir/nir_validate.c | 95 +++-
> 10 files changed, 1162 insertions(+), 699 deletions(-)
> create mode 100644 src/glsl/nir/nir_control_flow.c
> create mode 100644 src/glsl/nir/nir_control_flow.h
> create mode 100644 src/glsl/nir/nir_control_flow_private.h
FWIW, I just ran into a bug: splitting the start block doesn't update
impl->start_block. For example, trying to insert a nir_if in the middle
of the starting block would split it, but leave start_block pointing at
the later block - in the middle of the program. This wreaks havoc.
I'm not sure if this is a new bug or not, though.
(Connor mentioned on IRC that we could simply delete the start_block
field entirely, since it's just the first node in the list anyway, and
thus easily accessible without a special field that we have to keep
updated all the time...)
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 819 bytes
Desc: This is a digitally signed message part.
More information about the mesa-dev