[Mesa-dev] [PATCH v3 5/7] i965: Add a NIR analysis pass for determining when a boolean resolve is needed
Matt Turner
mattst88 at gmail.com
Fri Mar 20 15:41:26 PDT 2015
On Fri, Mar 20, 2015 at 2:23 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> v2: Fix the spelling of analyze and re-arrange code for better readability
> as per Connor's comments.
> v3: Make the naming of things more consistent and add a pile of comments
> ---
> src/mesa/drivers/dri/i965/Makefile.sources | 2 +
> src/mesa/drivers/dri/i965/brw_nir.h | 78 ++++++
> .../dri/i965/brw_nir_analyze_boolean_resolves.c | 275 +++++++++++++++++++++
> 3 files changed, 355 insertions(+)
> create mode 100644 src/mesa/drivers/dri/i965/brw_nir.h
> create mode 100644 src/mesa/drivers/dri/i965/brw_nir_analyze_boolean_resolves.c
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources b/src/mesa/drivers/dri/i965/Makefile.sources
> index c69441b..3a3df70 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -73,6 +73,8 @@ i965_FILES = \
> brw_meta_util.h \
> brw_misc_state.c \
> brw_multisample_state.h \
> + brw_nir.h \
> + brw_nir_analyze_boolean_resolves.c \
> brw_object_purgeable.c \
> brw_packed_float.c \
> brw_performance_monitor.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.h b/src/mesa/drivers/dri/i965/brw_nir.h
> new file mode 100644
> index 0000000..27782a3
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_nir.h
> @@ -0,0 +1,78 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#pragma once
> +
> +#include "glsl/nir/nir.h"
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/* Flags set in the instr->pass_flags field by i965 analysis passes */
> +enum {
> + BRW_NIR_NON_BOOLEAN = 0x0,
> +
> + /* Indicates that the given instruction's destination is a boolean
> + * value but that it needs to be resolved before it can be used.
> + * On Gen <= 5, CMP instructions return a 32-bit value where the bottom
> + * bit represents the actual true/false value of the compare and the top
> + * 31 bits are undefined. In order to use this value, we have to do a
> + * "resolve" operation by replacing the value of the CMP with -(x & 1)
> + * to sign-extend the bottom bit to 0/~0.
> + */
> + BRW_NIR_BOOLEAN_NEEDS_RESOLVE = 0x1,
> +
> + /* Indicates that the given instruction's destination is a boolean
> + * value that has intentionally been left unresolved. Not all boolean
> + * values need to be resolved immediately. For instance, if we have
> + *
> + * CMP r1 r2 r3
> + * CMP r4 r5 r6
> + * AND r7 r1 r4
> + *
> + * We don't have to resolve the result of the two CMP instructions
> + * immediately because the AND still does an AND of the bottom bits.
> + * Instead, we can save ourselves instructions by delaying the resolve
> + * until after the AND. The result of the two CMP instructions is left
> + * as BRW_NIR_BOOLEAN_UNRESOLVED.
> + */
> + BRW_NIR_BOOLEAN_UNRESOLVED = 0x2,
> +
> + /* Indicates a that the given instruction's destination is a boolean
> + * value that does not need a resolve. For instance, if you AND two
> + * values that are BRW_NIR_BOOLEAN_NEEDS_RESOLVE then we know that both
Ah, I misunderstood this comment on first read. You're saying that the
two things marked as NEEDS_RESOLVE will be resolved before the AND
consumes them, ... so the AND's destination will be marked with
NO_RESOLVE?
That makes sense.
> + * values will be 0/~0 before we get them and the result of the AND is
> + * also guaranteed to be 0/~0 and does not need a resolve.
> + */
> + BRW_NIR_BOOLEAN_NO_RESOLVE = 0x3,
> +
> + /* A mask to mask the boolean status values off of instr->pass_flags */
> + BRW_NIR_BOOLEAN_MASK = 0x3,
> +};
> +
> +void brw_nir_analyze_boolean_resolves(nir_shader *nir);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> diff --git a/src/mesa/drivers/dri/i965/brw_nir_analyze_boolean_resolves.c b/src/mesa/drivers/dri/i965/brw_nir_analyze_boolean_resolves.c
> new file mode 100644
> index 0000000..e893a65
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_nir_analyze_boolean_resolves.c
> @@ -0,0 +1,275 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Jason Ekstrand <jason at jlekstrand.net>
> + */
> +
> +#include "brw_nir.h"
> +
> +/*
> + * This file implements an analysis pass that determines when we have to do
> + * a boolean resolve on Gen <= 5. Instructions that need a boolean resolve
> + * will have the booleans portion of the instr->pass_flags field set to
> + * BRW_NIR_BOOLEAN_NEEDS_RESOLVE.
> + */
> +
> +
> +/** Returns the resolve status for the given source
> + *
> + * If the source has a parent instruction then the resolve status is the
> + * status of the parent instruction. If the source does not have a parent
> + * instruction then we don't know so we return NON_BOOLEAN.
> + */
> +static uint8_t
> +get_resolve_status_for_src(nir_src *src)
> +{
> + nir_instr *src_instr;
> + if (src->is_ssa) {
> + src_instr = src->ssa->parent_instr;
> + } else {
> + src_instr = src->reg.reg->parent_instr;
> + }
> +
> + if (src_instr) {
> + uint8_t resolve_status = src_instr->pass_flags & BRW_NIR_BOOLEAN_MASK;
> +
> + /* If the source instruction needs resolve, then from the perspective
> + * of the user, it's a true boolean.
> + */
> + if (resolve_status == BRW_NIR_BOOLEAN_NEEDS_RESOLVE)
> + resolve_status = BRW_NIR_BOOLEAN_NO_RESOLVE;
> + return resolve_status;
> + } else {
> + return BRW_NIR_NON_BOOLEAN;
> + }
> +}
> +
> +/** Marks the given source as needing a resolve
> + *
> + * If the given source corresponds to an unresolved boolean it marks it as
> + * needing a resolve. Otherwise, we leave it alone.
> + */
> +static bool
> +src_mark_needs_resolve(nir_src *src, void *void_state)
> +{
> + nir_instr *src_instr;
> + if (src->is_ssa) {
> + src_instr = src->ssa->parent_instr;
> + } else {
> + src_instr = src->reg.reg->parent_instr;
> + }
> +
> + if (src_instr) {
> + uint8_t resolve_status = src_instr->pass_flags & BRW_NIR_BOOLEAN_MASK;
> +
> + /* If the source instruction is unresolved, then mark it as needing
> + * to be resolved.
> + */
> + if (resolve_status == BRW_NIR_BOOLEAN_UNRESOLVED) {
> + src_instr->pass_flags &= ~BRW_NIR_BOOLEAN_MASK;
> + src_instr->pass_flags |= BRW_NIR_BOOLEAN_NEEDS_RESOLVE;
> + }
> +
> + }
> +
> + return true;
> +}
> +
> +static bool
> +analyze_boolean_resolves_block(nir_block *block, void *void_state)
> +{
> + nir_foreach_instr(block, instr) {
> + switch (instr->type) {
> + case nir_instr_type_alu: {
> + /* For ALU instructions, the resolve status is handled in a
> + * three-step process.
> + *
> + * 1) Look at the instruction type and sources and determine if it
> + * can be left unresolved.
> + *
> + * 2) Look at the destination and see if we have to resolve
> + * anyway. (This is the case if this instruction is not the
> + * only instruction writing to a given register.)
> + *
> + * 3) If the instruction has a resolve status other than
> + * BOOL_UNRESOLVED or BOOL_NEEDS_RESOLVE then we walk through
> + * the sources and ensure that they are also resolved. This
> + * ensures that we don't end up with any stray unresolved
> + * booleans going into ADDs or something like that.
> + */
> +
> + uint8_t resolve_status;
> + nir_alu_instr *alu = nir_instr_as_alu(instr);
> + switch (alu->op) {
> + case nir_op_flt:
> + case nir_op_ilt:
> + case nir_op_ult:
> + case nir_op_fge:
> + case nir_op_ige:
> + case nir_op_uge:
> + case nir_op_feq:
> + case nir_op_ieq:
> + case nir_op_fne:
> + case nir_op_ine:
> + case nir_op_f2b:
> + case nir_op_i2b:
> + /* This instruction will turn into a CMP when we actually emit
> + * so the result will have to be resolved before it can be used.
> + */
> + resolve_status = BRW_NIR_BOOLEAN_UNRESOLVED;
> +
> + /* Even though the destination is allowed to be left unresolved,
> + * the sources are treated as regular integers or floats so
> + * they need to be resolved.
> + */
> + nir_foreach_src(instr, src_mark_needs_resolve, NULL);
> + break;
> +
> + case nir_op_imov:
> + case nir_op_inot:
> + if (alu->dest.write_mask == 1) {
> + /* This is a single-source instruction. Just copy the
> + * resolve status from the source.
> + */
> + resolve_status = get_resolve_status_for_src(&alu->src[0].src);
> + } else {
> + /* Vector versions will be assumed to be non-boolean. */
> + resolve_status = BRW_NIR_NON_BOOLEAN;
I've got some patches to allow ir_unop_logic_not (and and, or, xor) to
operate per-component. That's not related to this comment though,
right?
As far as I can tell this looks good. It'd be cool if Connor could confirm.
Reviewed-by: Matt Turner <mattst88 at gmail.com>
More information about the mesa-dev
mailing list