[Mesa-dev] [PATCH 1/5] nir: split out instruction comparison functions
Connor Abbott
cwabbott0 at gmail.com
Wed Sep 23 23:44:52 PDT 2015
On Tue, Aug 18, 2015 at 1:29 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> On May 22, 2015 11:25 AM, "Connor Abbott" <cwabbott0 at gmail.com> wrote:
>>
>> We'll want to use these outside of nir_opt_cse.c. Rather than adding yet
>> another thing to the mess that is nir.c, create a new file and move
>> nir_srcs_equal() there too.
>>
>> Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
>> ---
>> src/glsl/Makefile.sources | 1 +
>> src/glsl/nir/nir.c | 27 ------
>> src/glsl/nir/nir.h | 1 +
>> src/glsl/nir/nir_instr_compare.c | 179
>> +++++++++++++++++++++++++++++++++++++++
>> src/glsl/nir/nir_opt_cse.c | 121 --------------------------
>> 5 files changed, 181 insertions(+), 148 deletions(-)
>> create mode 100644 src/glsl/nir/nir_instr_compare.c
>>
>> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
>> index d784a81..75e5377 100644
>> --- a/src/glsl/Makefile.sources
>> +++ b/src/glsl/Makefile.sources
>> @@ -27,6 +27,7 @@ NIR_FILES = \
>> nir/nir_constant_expressions.h \
>> nir/nir_dominance.c \
>> nir/nir_from_ssa.c \
>> + nir/nir_instr_compare.c \
>> nir/nir_intrinsics.c \
>> nir/nir_intrinsics.h \
>> nir/nir_live_variables.c \
>> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
>> index f03e80a..170807e 100644
>> --- a/src/glsl/nir/nir.c
>> +++ b/src/glsl/nir/nir.c
>> @@ -1785,33 +1785,6 @@ nir_src_as_const_value(nir_src src)
>> return &load->value;
>> }
>>
>> -bool
>> -nir_srcs_equal(nir_src src1, nir_src src2)
>> -{
>> - if (src1.is_ssa) {
>> - if (src2.is_ssa) {
>> - return src1.ssa == src2.ssa;
>> - } else {
>> - return false;
>> - }
>> - } else {
>> - if (src2.is_ssa) {
>> - return false;
>> - } else {
>> - if ((src1.reg.indirect == NULL) != (src2.reg.indirect == NULL))
>> - return false;
>> -
>> - if (src1.reg.indirect) {
>> - if (!nir_srcs_equal(*src1.reg.indirect, *src2.reg.indirect))
>> - return false;
>> - }
>> -
>> - return src1.reg.reg == src2.reg.reg &&
>> - src1.reg.base_offset == src2.reg.base_offset;
>> - }
>> - }
>> -}
>> -
>> static bool
>> src_is_valid(const nir_src *src)
>> {
>> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
>> index 697d37e..f04195f 100644
>> --- a/src/glsl/nir/nir.h
>> +++ b/src/glsl/nir/nir.h
>> @@ -1579,6 +1579,7 @@ bool nir_foreach_src(nir_instr *instr,
>> nir_foreach_src_cb cb, void *state);
>>
>> nir_const_value *nir_src_as_const_value(nir_src src);
>> bool nir_srcs_equal(nir_src src1, nir_src src2);
>> +bool nir_instrs_equal(nir_instr *instr1, nir_instr *instr2);
>> void nir_instr_rewrite_src(nir_instr *instr, nir_src *src, nir_src
>> new_src);
>> void nir_instr_move_src(nir_instr *dest_instr, nir_src *dest, nir_src
>> *src);
>> void nir_if_rewrite_condition(nir_if *if_stmt, nir_src new_src);
>> diff --git a/src/glsl/nir/nir_instr_compare.c
>> b/src/glsl/nir/nir_instr_compare.c
>> new file mode 100644
>> index 0000000..89b576c
>> --- /dev/null
>> +++ b/src/glsl/nir/nir_instr_compare.c
>> @@ -0,0 +1,179 @@
>> +/*
>> + * Copyright © 2014 Intel Corporation
>> + * Copyright © 2015 Connor Abbott
>> + *
>> + * 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)
>> + * Connor Abbott (cwabbott0 at gmail.com)
>> + *
>> + */
>> +
>> +#include "nir.h"
>> +
>> +bool
>> +nir_srcs_equal(nir_src src1, nir_src src2)
>> +{
>> + if (src1.is_ssa) {
>> + if (src2.is_ssa) {
>> + return src1.ssa == src2.ssa;
>> + } else {
>> + return false;
>> + }
>> + } else {
>> + if (src2.is_ssa) {
>> + return false;
>> + } else {
>> + if ((src1.reg.indirect == NULL) != (src2.reg.indirect == NULL))
>> + return false;
>> +
>> + if (src1.reg.indirect) {
>> + if (!nir_srcs_equal(*src1.reg.indirect, *src2.reg.indirect))
>> + return false;
>> + }
>> +
>> + return src1.reg.reg == src2.reg.reg &&
>> + src1.reg.base_offset == src2.reg.base_offset;
>> + }
>> + }
>> +}
>> +
>> +
>> +static bool
>> +nir_alu_srcs_equal(nir_alu_instr *alu1, nir_alu_instr *alu2, unsigned
>> src1,
>> + unsigned src2)
>> +{
>> + if (alu1->src[src1].abs != alu2->src[src2].abs ||
>> + alu1->src[src1].negate != alu2->src[src2].negate)
>> + return false;
>> +
>> + for (unsigned i = 0; i < nir_ssa_alu_instr_src_components(alu1, src1);
>> i++) {
>> + if (alu1->src[src1].swizzle[i] != alu2->src[src2].swizzle[i])
>> + return false;
>> + }
>> +
>> + return nir_srcs_equal(alu1->src[src1].src, alu2->src[src2].src);
>> +}
>
> We should add a big fat disclaimer here that just because this function
> returns false doesn't mean they aren't equal. nir_instrs_equal(instr,
> instr) may return false.
Actually, it won't. It may assert fail if you give it something that
it can't handle, but it should never return false. There was one
instance where it could return false that Eric added with the
texturing comparison code, but that's a bug and I've fixed it in my
new series. If you think about it, we really don't want a case where
nir_instrs_equal(instr, instr) returns false, since then we could
insert an instruction into the hash set but then never be able to find
it, which is silly and wasteful.
>
> Also, does this need to be rebased on Eric's stuff?
>
>> +bool
>> +nir_instrs_equal(nir_instr *instr1, nir_instr *instr2)
>> +{
>> + if (instr1->type != instr2->type)
>> + return false;
>> +
>> + switch (instr1->type) {
>> + case nir_instr_type_alu: {
>> + nir_alu_instr *alu1 = nir_instr_as_alu(instr1);
>> + nir_alu_instr *alu2 = nir_instr_as_alu(instr2);
>> +
>> + if (alu1->op != alu2->op)
>> + return false;
>> +
>> + /* TODO: We can probably acutally do something more inteligent such
>> + * as allowing different numbers and taking a maximum or something
>> + * here */
>> + if (alu1->dest.dest.ssa.num_components !=
>> alu2->dest.dest.ssa.num_components)
>> + return false;
>> +
>> + if (nir_op_infos[alu1->op].algebraic_properties &
>> NIR_OP_IS_COMMUTATIVE) {
>> + assert(nir_op_infos[alu1->op].num_inputs == 2);
>> + return (nir_alu_srcs_equal(alu1, alu2, 0, 0) &&
>> + nir_alu_srcs_equal(alu1, alu2, 1, 1)) ||
>> + (nir_alu_srcs_equal(alu1, alu2, 0, 1) &&
>> + nir_alu_srcs_equal(alu1, alu2, 1, 0));
>> + } else {
>> + for (unsigned i = 0; i < nir_op_infos[alu1->op].num_inputs; i++)
>> {
>> + if (!nir_alu_srcs_equal(alu1, alu2, i, i))
>> + return false;
>> + }
>> + }
>> + return true;
>> + }
>> + case nir_instr_type_tex:
>> + return false;
>> + case nir_instr_type_load_const: {
>> + nir_load_const_instr *load1 = nir_instr_as_load_const(instr1);
>> + nir_load_const_instr *load2 = nir_instr_as_load_const(instr2);
>> +
>> + if (load1->def.num_components != load2->def.num_components)
>> + return false;
>> +
>> + return memcmp(load1->value.f, load2->value.f,
>> + load1->def.num_components * sizeof(*load2->value.f))
>> == 0;
>> + }
>> + case nir_instr_type_phi: {
>> + nir_phi_instr *phi1 = nir_instr_as_phi(instr1);
>> + nir_phi_instr *phi2 = nir_instr_as_phi(instr2);
>> +
>> + if (phi1->instr.block != phi2->instr.block)
>> + return false;
>> +
>> + nir_foreach_phi_src(phi1, src1) {
>> + nir_foreach_phi_src(phi2, src2) {
>> + if (src1->pred == src2->pred) {
>> + if (!nir_srcs_equal(src1->src, src2->src))
>> + return false;
>> +
>> + break;
>> + }
>> + }
>> + }
>> +
>> + return true;
>> + }
>> + case nir_instr_type_intrinsic: {
>> + nir_intrinsic_instr *intrinsic1 = nir_instr_as_intrinsic(instr1);
>> + nir_intrinsic_instr *intrinsic2 = nir_instr_as_intrinsic(instr2);
>> + const nir_intrinsic_info *info =
>> + &nir_intrinsic_infos[intrinsic1->intrinsic];
>> +
>> + if (intrinsic1->intrinsic != intrinsic2->intrinsic ||
>> + intrinsic1->num_components != intrinsic2->num_components)
>> + return false;
>> +
>> + if (info->has_dest && intrinsic1->dest.ssa.num_components !=
>> + intrinsic2->dest.ssa.num_components)
>> + return false;
>> +
>> + for (unsigned i = 0; i < info->num_srcs; i++) {
>> + if (!nir_srcs_equal(intrinsic1->src[i], intrinsic2->src[i]))
>> + return false;
>> + }
>> +
>> + assert(info->num_variables == 0);
>> +
>> + for (unsigned i = 0; i < info->num_indices; i++) {
>> + if (intrinsic1->const_index[i] != intrinsic2->const_index[i])
>> + return false;
>> + }
>> +
>> + return true;
>> + }
>> + case nir_instr_type_call:
>> + case nir_instr_type_jump:
>> + case nir_instr_type_ssa_undef:
>> + case nir_instr_type_parallel_copy:
>> + default:
>> + unreachable("Invalid instruction type");
>> + }
>> +
>> + return false;
>> +}
>> diff --git a/src/glsl/nir/nir_opt_cse.c b/src/glsl/nir/nir_opt_cse.c
>> index 553906e..a6a4a21 100644
>> --- a/src/glsl/nir/nir_opt_cse.c
>> +++ b/src/glsl/nir/nir_opt_cse.c
>> @@ -37,127 +37,6 @@ struct cse_state {
>> };
>>
>> static bool
>> -nir_alu_srcs_equal(nir_alu_instr *alu1, nir_alu_instr *alu2, unsigned
>> src1,
>> - unsigned src2)
>> -{
>> - if (alu1->src[src1].abs != alu2->src[src2].abs ||
>> - alu1->src[src1].negate != alu2->src[src2].negate)
>> - return false;
>> -
>> - for (unsigned i = 0; i < nir_ssa_alu_instr_src_components(alu1, src1);
>> i++) {
>> - if (alu1->src[src1].swizzle[i] != alu2->src[src2].swizzle[i])
>> - return false;
>> - }
>> -
>> - return nir_srcs_equal(alu1->src[src1].src, alu2->src[src2].src);
>> -}
>> -
>> -static bool
>> -nir_instrs_equal(nir_instr *instr1, nir_instr *instr2)
>> -{
>> - if (instr1->type != instr2->type)
>> - return false;
>> -
>> - switch (instr1->type) {
>> - case nir_instr_type_alu: {
>> - nir_alu_instr *alu1 = nir_instr_as_alu(instr1);
>> - nir_alu_instr *alu2 = nir_instr_as_alu(instr2);
>> -
>> - if (alu1->op != alu2->op)
>> - return false;
>> -
>> - /* TODO: We can probably acutally do something more inteligent such
>> - * as allowing different numbers and taking a maximum or something
>> - * here */
>> - if (alu1->dest.dest.ssa.num_components !=
>> alu2->dest.dest.ssa.num_components)
>> - return false;
>> -
>> - if (nir_op_infos[alu1->op].algebraic_properties &
>> NIR_OP_IS_COMMUTATIVE) {
>> - assert(nir_op_infos[alu1->op].num_inputs == 2);
>> - return (nir_alu_srcs_equal(alu1, alu2, 0, 0) &&
>> - nir_alu_srcs_equal(alu1, alu2, 1, 1)) ||
>> - (nir_alu_srcs_equal(alu1, alu2, 0, 1) &&
>> - nir_alu_srcs_equal(alu1, alu2, 1, 0));
>> - } else {
>> - for (unsigned i = 0; i < nir_op_infos[alu1->op].num_inputs; i++)
>> {
>> - if (!nir_alu_srcs_equal(alu1, alu2, i, i))
>> - return false;
>> - }
>> - }
>> - return true;
>> - }
>> - case nir_instr_type_tex:
>> - return false;
>> - case nir_instr_type_load_const: {
>> - nir_load_const_instr *load1 = nir_instr_as_load_const(instr1);
>> - nir_load_const_instr *load2 = nir_instr_as_load_const(instr2);
>> -
>> - if (load1->def.num_components != load2->def.num_components)
>> - return false;
>> -
>> - return memcmp(load1->value.f, load2->value.f,
>> - load1->def.num_components * sizeof(*load2->value.f))
>> == 0;
>> - }
>> - case nir_instr_type_phi: {
>> - nir_phi_instr *phi1 = nir_instr_as_phi(instr1);
>> - nir_phi_instr *phi2 = nir_instr_as_phi(instr2);
>> -
>> - if (phi1->instr.block != phi2->instr.block)
>> - return false;
>> -
>> - nir_foreach_phi_src(phi1, src1) {
>> - nir_foreach_phi_src(phi2, src2) {
>> - if (src1->pred == src2->pred) {
>> - if (!nir_srcs_equal(src1->src, src2->src))
>> - return false;
>> -
>> - break;
>> - }
>> - }
>> - }
>> -
>> - return true;
>> - }
>> - case nir_instr_type_intrinsic: {
>> - nir_intrinsic_instr *intrinsic1 = nir_instr_as_intrinsic(instr1);
>> - nir_intrinsic_instr *intrinsic2 = nir_instr_as_intrinsic(instr2);
>> - const nir_intrinsic_info *info =
>> - &nir_intrinsic_infos[intrinsic1->intrinsic];
>> -
>> - if (intrinsic1->intrinsic != intrinsic2->intrinsic ||
>> - intrinsic1->num_components != intrinsic2->num_components)
>> - return false;
>> -
>> - if (info->has_dest && intrinsic1->dest.ssa.num_components !=
>> - intrinsic2->dest.ssa.num_components)
>> - return false;
>> -
>> - for (unsigned i = 0; i < info->num_srcs; i++) {
>> - if (!nir_srcs_equal(intrinsic1->src[i], intrinsic2->src[i]))
>> - return false;
>> - }
>> -
>> - assert(info->num_variables == 0);
>> -
>> - for (unsigned i = 0; i < info->num_indices; i++) {
>> - if (intrinsic1->const_index[i] != intrinsic2->const_index[i])
>> - return false;
>> - }
>> -
>> - return true;
>> - }
>> - case nir_instr_type_call:
>> - case nir_instr_type_jump:
>> - case nir_instr_type_ssa_undef:
>> - case nir_instr_type_parallel_copy:
>> - default:
>> - unreachable("Invalid instruction type");
>> - }
>> -
>> - return false;
>> -}
>> -
>> -static bool
>> src_is_ssa(nir_src *src, void *data)
>> {
>> (void) data;
>> --
>> 2.1.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list