[Mesa-dev] [PATCH v3 5/7] i965: Add a NIR analysis pass for determining when a boolean resolve is needed
Jason Ekstrand
jason at jlekstrand.net
Fri Mar 20 16:13:15 PDT 2015
On Fri, Mar 20, 2015 at 3:41 PM, Matt Turner <mattst88 at gmail.com> wrote:
> 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?
Right. Those should get lowered by alu-to-scalar right now. I guess
it might matter in vec4 and it should be safe for vectors. It just
makes NIR -> FS a little more complicated. I can change that if you'd
like.
> 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