On Tuesday, July 21, 2015 07:54:34 PM Connor Abbott wrote:
> These will help us do a number of things, including:
>
> - Early return elimination.
> - Dead control flow elimination.
> - Various optimizations, such as replacing:
>
> if (foo) {
> ...
> }
> if (!foo) {
> ...
> }
>
> with:
>
> if (foo) {
> ...
> } else {
> ...
> }
>
> Signed-off-by: Connor Abbott <connor.w.abb...@intel.com>
> ---
> src/glsl/nir/nir_control_flow.c | 62 ++++++++++++++++++++++++++++++++++
> src/glsl/nir/nir_control_flow.h | 75
> +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 137 insertions(+)
>
> diff --git a/src/glsl/nir/nir_control_flow.c b/src/glsl/nir/nir_control_flow.c
> index ba39205..7864138 100644
> --- a/src/glsl/nir/nir_control_flow.c
> +++ b/src/glsl/nir/nir_control_flow.c
> @@ -739,3 +739,65 @@ nir_cf_node_remove(nir_cf_node *node)
> cleanup_cf_node(node, impl);
> }
>
> +void
> +nir_cf_extract(nir_cf_list *extracted, nir_cursor begin, nir_cursor end)
> +{
> + nir_block *block_begin, *block_end, *block_before, *block_after;
> +
> + /* In the case where begin points to an instruction in some basic block
> and
> + * end points to the end of the same basic block, we rely on the fact that
> + * splitting on an instruction moves earlier instructions into a new basic
> + * block. If the later instructions were moved instead, then the end
> cursor
> + * would be pointing to the same place that begin used to point to, which
> + * is obviously not what we want.
> + */
> + split_block_cursor(begin, &block_before, &block_begin);
> + split_block_cursor(end, &block_end, &block_after);
> +
> + extracted->impl = nir_cf_node_get_function(&block_begin->cf_node);
> + exec_list_make_empty(&extracted->list);
> +
> + nir_cf_node *cf_node = &block_begin->cf_node;
> + nir_cf_node *cf_node_end = &block_end->cf_node;
> + while (true) {
> + nir_cf_node *next = nir_cf_node_next(cf_node);
> +
> + exec_node_remove(&cf_node->node);
> + cf_node->parent = NULL;
> + exec_list_push_tail(&extracted->list, &cf_node->node);
> +
> + if (cf_node == cf_node_end)
> + break;
> +
> + cf_node = next;
> + }
> +
> + stitch_blocks(block_before, block_after);
> +}
> +
> +void
> +nir_cf_reinsert(nir_cf_list *cf_list, nir_cursor cursor)
> +{
> + nir_block *before, *after;
> +
> + split_block_cursor(cursor, &before, &after);
> +
> + foreach_list_typed_safe(nir_cf_node, node, node, &cf_list->list) {
> + exec_node_remove(&node->node);
> + node->parent = before->cf_node.parent;
> + exec_node_insert_node_before(&after->cf_node.node, &node->node);
> + }
> +
> + stitch_blocks(before,
> + nir_cf_node_as_block(nir_cf_node_next(&before->cf_node)));
> + stitch_blocks(nir_cf_node_as_block(nir_cf_node_prev(&after->cf_node)),
> + after);
> +}
> +
> +void
> +nir_cf_delete(nir_cf_list *cf_list)
> +{
> + foreach_list_typed(nir_cf_node, node, node, &cf_list->list) {
> + cleanup_cf_node(node, cf_list->impl);
> + }
> +}
> diff --git a/src/glsl/nir/nir_control_flow.h b/src/glsl/nir/nir_control_flow.h
> index 5a3be26..45aff3e 100644
> --- a/src/glsl/nir/nir_control_flow.h
> +++ b/src/glsl/nir/nir_control_flow.h
> @@ -37,6 +37,12 @@ extern "C" {
> *
> * This file contains various API's that make modifying control flow in NIR,
> * while maintaining the invariants checked by the validator, much easier.
> + * There are two parts to this:
> + *
> + * 1. Inserting control flow (if's and loops) in various places, for creating
> + * IR either from scratch or as part of some lowering pass.
> + * 2. Taking existing pieces of the IR and either moving them around or
> + * deleting them.
> */
>
> /* Helper struct for representing a point to extract/insert. Helps reduce the
> @@ -164,6 +170,75 @@ nir_cf_node_insert_end(struct exec_list *list,
> nir_cf_node *node)
> /** removes a control flow node, doing any cleanup necessary */
> void nir_cf_node_remove(nir_cf_node *node);
>
> +/** Control flow motion.
> + *
> + * These functions let you take a part of a control flow list (basically
> + * equivalent to a series of statement in GLSL) and "extract" it from the IR,
> + * so that it's a free-floating piece of IR that can be either re-inserted
> + * somewhere else or deleted entirely. A few notes on using it:
> + *
> + * 1. Phi nodes are considered attached to the piece of control flow that
> + * their sources come from. There are three places where phi nodes can
> + * occur, which are the three places where a block can have multiple
> + * predecessors:
> + *
> + * 1) After an if statement, if neither branch ends in a jump.
> + * 2) After a loop, if there are multiple break's.
> + * 3) At the beginning of a loop.
> + *
> + * For #1, the phi node is considered to be part of the if, and for #2 and
> + * #3 the phi node is considered to be part of the loop. This allows us to
> + * keep phi's intact, but it means that phi nodes cannot be separated from
> + * the control flow they come from. For example, extracting an if without
> + * extracting all the phi nodes after it is not allowed, and neither is
> + * extracting only some of the phi nodes at the beginning of a block. It
> + * also means that extracting from the beginning of a basic block actually
> + * means extracting from the first non-phi instruction, since there's no
> + * situation where extracting phi nodes without extracting what comes
> + * before them makes any sense.
> + *
> + * 2. Phi node sources are guaranteed to remain valid, meaning that they
> still
> + * correspond one-to-one with the predecessors of the basic block they're
> + * part of. In addition, the original sources will be preserved unless
> they
> + * correspond to a break or continue that was deleted. However, no attempt
> + * is made to ensure that SSA form is maintained. In particular, it is
> + * *not* guaranteed that definitions of SSA values will dominate all their
> + * uses after all is said and done. Either the caller must ensure that
> this
> + * is the case, or it must insert extra phi nodes to restore SSA.
> + *
> + * 3. It is invalid to move a piece of IR with a break/continue outside of
> the
> + * loop it references. Doing this will result in invalid
> + * successors/predecessors and phi node sources.
> + *
> + * 4. It is invalid to move a piece of IR from one function implementation to
> + * another.
> + *
> + * 5. Extracting a control flow list will leave lots of dangling references
> to
> + * and from other pieces of the IR. It also leaves things in a not 100%
> + * consistent state. This means that some things (e.g. inserting
> + * instructions) might not work reliably on the extracted control flow. It
> + * also means that extracting control flow without re-inserting it or
> + * deleting it is a Bad Thing (tm).
> + */
> +
> +typedef struct {
> + struct exec_list list;
> + nir_function_impl *impl; /* for cleaning up if the list is deleted */
> +} nir_cf_list;
> +
> +void nir_cf_extract(nir_cf_list *extracted, nir_cursor begin, nir_cursor
> end);
> +
> +void nir_cf_reinsert(nir_cf_list *cf_list, nir_cursor cursor);
> +
> +void nir_cf_delete(nir_cf_list *cf_list);
> +
> +static inline void
> +nir_cf_list_extract(nir_cf_list *extracted, struct exec_list *cf_list)
> +{
> + nir_cf_extract(extracted, nir_before_cf_list(cf_list),
> + nir_after_cf_list(cf_list));
> +}
> +
> #ifdef __cplusplus
> }
> #endif
> Given all of these restrictions, I'd feel a lot better about using this if nir_validate ensured that SSA is correct - phi definitions dominate their sources, and there aren't dangling references to bogus blocks, and so on. This seems like a reasonable tool - one simply must exercise caution when wielding it. Which is fine, especially if we put some other safeguards in place. I like the simplicity - this is surprisingly straightforward. The cursor idea helps a lot as well. I was wondering how it worked if there were no blocks, but then I remembered that there are padding blocks after if statements and loops. So that takes care of that. Reviewed-by: Kenneth Graunke <kenn...@whitecape.org> Feel free to land this series. Now to review the control flow optimization series...
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
