Re: [Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation
On Wed, 2018-10-31 at 13:22 +1100, Timothy Arceri wrote:
> On 31/10/18 1:23 am, Jason Ekstrand wrote:
> > Weird. I didn't expect this patch to have any impact whatsoever.
> > I
> > thought it was just moving around the way we emit stuff.
>
> I think I've spotted the problem. Iago does patch 1 help with the
> regressions you are seeing.
>
> https://patchwork.freedesktop.org/series/51789/
Yes, patch 1 fixes the problem. Thanks Timothy!
Iago
> >
> > On October 30, 2018 08:40:01 Iago Toral wrote:
> >
> > > Jason, JFYI, I have been looking into the cases where the boolean
> > > bitsize lowering pass was producing worse instruction counts that
> > > the
> > > default 32-bit pass and I have tracked it down to this patch.
> > > Reverting
> > > this makes the instruction count much better for some tests, I'll
> > > check
> > > why this happens tomorrow.
> > >
> > > Iago
> > >
> > > On Mon, 2018-10-22 at 17:13 -0500, Jason Ekstrand wrote:
> > > > Instead of doing our own constant folding, we just emit
> > > > instructions
> > > > and
> > > > let constant folding happen. This is substantially simpler and
> > > > lets
> > > > us
> > > > use the nir_imm_bool helper instead of dealing with the
> > > > const_value's
> > > > ourselves.
> > > > ---
> > > > src/compiler/nir/nir_opt_if.c | 91 ---
> > > > --
> > > > --
> > > > 1 file changed, 30 insertions(+), 61 deletions(-)
> > > >
> > > > diff --git a/src/compiler/nir/nir_opt_if.c
> > > > b/src/compiler/nir/nir_opt_if.c
> > > > index 0c94aa170b5..60368a0259e 100644
> > > > --- a/src/compiler/nir/nir_opt_if.c
> > > > +++ b/src/compiler/nir/nir_opt_if.c
> > > > @@ -377,31 +377,15 @@ opt_if_loop_terminator(nir_if *nif)
> > > > return true;
> > > > }
> > > >
> > > > -static void
> > > > -replace_if_condition_use_with_const(nir_builder *b, nir_src
> > > > *use,
> > > > -nir_const_value
> > > > nir_boolean,
> > > > -bool if_condition)
> > > > -{
> > > > - /* Create const */
> > > > - nir_ssa_def *const_def = nir_build_imm(b, 1, 32,
> > > > nir_boolean);
> > > > -
> > > > - /* Rewrite use to use const */
> > > > - nir_src new_src = nir_src_for_ssa(const_def);
> > > > - if (if_condition)
> > > > - nir_if_rewrite_condition(use->parent_if, new_src);
> > > > - else
> > > > - nir_instr_rewrite_src(use->parent_instr, use, new_src);
> > > > -}
> > > > -
> > > > static bool
> > > > -evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t
> > > > *value)
> > > > +evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool
> > > > *value)
> > > > {
> > > > nir_block *use_block = nir_cursor_current_block(cursor);
> > > > if (nir_block_dominates(nir_if_first_then_block(nif),
> > > > use_block))
> > > > {
> > > > - *value = NIR_TRUE;
> > > > + *value = true;
> > > >return true;
> > > > } else if
> > > > (nir_block_dominates(nir_if_first_else_block(nif),
> > > > use_block)) {
> > > > - *value = NIR_FALSE;
> > > > + *value = false;
> > > >return true;
> > > > } else {
> > > >return false;
> > > > @@ -460,52 +444,31 @@ propagate_condition_eval(nir_builder *b,
> > > > nir_if
> > > > *nif, nir_src *use_src,
> > > > nir_src *alu_use, nir_alu_instr *alu,
> > > > bool is_if_condition)
> > > > {
> > > > - bool progress = false;
> > > > + bool bool_value;
> > > > + if (!evaluate_if_condition(nif, b->cursor, &bool_value))
> > > > + return false;
> > > >
> > > > - nir_const_value bool_value;
> > > > b->cursor = nir_before_src(alu_use, is_if_condition);
> > > > - if (nir_op_infos[alu->op].num_inputs == 1) {
> > > > - assert(alu->op == nir_op_inot || alu->op == nir_op_b2i);
> > > > -
> > > > - if (evaluate_if_condition(nif, b->cursor,
> > > > &bool_value.u32[0]))
> > > > {
> > > > - assert(nir_src_bit_size(alu->src[0].src) == 32);
> > > > -
> > > > - nir_const_value result =
> > > > -nir_eval_const_opcode(alu->op, 1, 32,
> > > > &bool_value);
> > > >
> > > > - replace_if_condition_use_with_const(b, alu_use,
> > > > result,
> > > > - is_if_condition);
> > > > - progress = true;
> > > > + nir_ssa_def *def[2] = { };
> > > > + for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs;
> > > > i++) {
> > > > + if (alu->src[i].src.ssa == use_src->ssa) {
> > > > + def[i] = nir_imm_bool(b, bool_value);
> > > > + } else {
> > > > + def[i] = alu->src[i].src.ssa;
> > > >}
> > > > - } else {
> > > > - assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
> > > > -
> > > > - if (evaluate_if_condition(nif, b->cursor,
> > > > &bool_value.u32[0]))
> > > > {
> > > > - nir_ssa_def *def[2];
> > > > - for (unsigned i = 0; i < 2; i++) {
> > > > -if (alu->src[i].src.ssa
Re: [Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation
On 31/10/18 1:23 am, Jason Ekstrand wrote:
Weird. I didn't expect this patch to have any impact whatsoever. I
thought it was just moving around the way we emit stuff.
I think I've spotted the problem. Iago does patch 1 help with the
regressions you are seeing.
https://patchwork.freedesktop.org/series/51789/
On October 30, 2018 08:40:01 Iago Toral wrote:
Jason, JFYI, I have been looking into the cases where the boolean
bitsize lowering pass was producing worse instruction counts that the
default 32-bit pass and I have tracked it down to this patch. Reverting
this makes the instruction count much better for some tests, I'll check
why this happens tomorrow.
Iago
On Mon, 2018-10-22 at 17:13 -0500, Jason Ekstrand wrote:
Instead of doing our own constant folding, we just emit instructions
and
let constant folding happen. This is substantially simpler and lets
us
use the nir_imm_bool helper instead of dealing with the const_value's
ourselves.
---
src/compiler/nir/nir_opt_if.c | 91 -
--
1 file changed, 30 insertions(+), 61 deletions(-)
diff --git a/src/compiler/nir/nir_opt_if.c
b/src/compiler/nir/nir_opt_if.c
index 0c94aa170b5..60368a0259e 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -377,31 +377,15 @@ opt_if_loop_terminator(nir_if *nif)
return true;
}
-static void
-replace_if_condition_use_with_const(nir_builder *b, nir_src *use,
- nir_const_value nir_boolean,
- bool if_condition)
-{
- /* Create const */
- nir_ssa_def *const_def = nir_build_imm(b, 1, 32, nir_boolean);
-
- /* Rewrite use to use const */
- nir_src new_src = nir_src_for_ssa(const_def);
- if (if_condition)
- nir_if_rewrite_condition(use->parent_if, new_src);
- else
- nir_instr_rewrite_src(use->parent_instr, use, new_src);
-}
-
static bool
-evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t
*value)
+evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value)
{
nir_block *use_block = nir_cursor_current_block(cursor);
if (nir_block_dominates(nir_if_first_then_block(nif), use_block))
{
- *value = NIR_TRUE;
+ *value = true;
return true;
} else if (nir_block_dominates(nir_if_first_else_block(nif),
use_block)) {
- *value = NIR_FALSE;
+ *value = false;
return true;
} else {
return false;
@@ -460,52 +444,31 @@ propagate_condition_eval(nir_builder *b, nir_if
*nif, nir_src *use_src,
nir_src *alu_use, nir_alu_instr *alu,
bool is_if_condition)
{
- bool progress = false;
+ bool bool_value;
+ if (!evaluate_if_condition(nif, b->cursor, &bool_value))
+ return false;
- nir_const_value bool_value;
b->cursor = nir_before_src(alu_use, is_if_condition);
- if (nir_op_infos[alu->op].num_inputs == 1) {
- assert(alu->op == nir_op_inot || alu->op == nir_op_b2i);
-
- if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0]))
{
- assert(nir_src_bit_size(alu->src[0].src) == 32);
-
- nir_const_value result =
- nir_eval_const_opcode(alu->op, 1, 32, &bool_value);
- replace_if_condition_use_with_const(b, alu_use, result,
- is_if_condition);
- progress = true;
+ nir_ssa_def *def[2] = { };
+ for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
+ if (alu->src[i].src.ssa == use_src->ssa) {
+ def[i] = nir_imm_bool(b, bool_value);
+ } else {
+ def[i] = alu->src[i].src.ssa;
}
- } else {
- assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
-
- if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0]))
{
- nir_ssa_def *def[2];
- for (unsigned i = 0; i < 2; i++) {
- if (alu->src[i].src.ssa == use_src->ssa) {
- def[i] = nir_build_imm(b, 1, 32, bool_value);
- } else {
- def[i] = alu->src[i].src.ssa;
- }
- }
-
- nir_ssa_def *nalu =
- nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
-
- /* Rewrite use to use new alu instruction */
- nir_src new_src = nir_src_for_ssa(nalu);
+ }
+ nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1],
NULL, NULL);
- if (is_if_condition)
- nir_if_rewrite_condition(alu_use->parent_if, new_src);
- else
- nir_instr_rewrite_src(alu_use->parent_instr, alu_use,
new_src);
+ /* Rewrite use to use new alu instruction */
+ nir_src new_src = nir_src_for_ssa(nalu);
- progress = true;
- }
- }
+ if (is_if_condition)
+ nir_if_rewrite_condition(alu_use->parent_if, new_src);
+ else
+ nir_instr_rewrite_src(alu_use->parent_instr, alu_use,
new_src);
- return progress;
+ return true;
}
static bool
@@ -527,11 +490,17 @@ evaluate_condition_use(nir_builder *b, nir_if
*nif,
Re: [Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation
Weird. I didn't expect this patch to have any impact whatsoever. I thought
it was just moving around the way we emit stuff.
On October 30, 2018 08:40:01 Iago Toral wrote:
Jason, JFYI, I have been looking into the cases where the boolean
bitsize lowering pass was producing worse instruction counts that the
default 32-bit pass and I have tracked it down to this patch. Reverting
this makes the instruction count much better for some tests, I'll check
why this happens tomorrow.
Iago
On Mon, 2018-10-22 at 17:13 -0500, Jason Ekstrand wrote:
Instead of doing our own constant folding, we just emit instructions
and
let constant folding happen. This is substantially simpler and lets
us
use the nir_imm_bool helper instead of dealing with the const_value's
ourselves.
---
src/compiler/nir/nir_opt_if.c | 91 -
--
1 file changed, 30 insertions(+), 61 deletions(-)
diff --git a/src/compiler/nir/nir_opt_if.c
b/src/compiler/nir/nir_opt_if.c
index 0c94aa170b5..60368a0259e 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -377,31 +377,15 @@ opt_if_loop_terminator(nir_if *nif)
return true;
}
-static void
-replace_if_condition_use_with_const(nir_builder *b, nir_src *use,
-nir_const_value nir_boolean,
-bool if_condition)
-{
- /* Create const */
- nir_ssa_def *const_def = nir_build_imm(b, 1, 32, nir_boolean);
-
- /* Rewrite use to use const */
- nir_src new_src = nir_src_for_ssa(const_def);
- if (if_condition)
- nir_if_rewrite_condition(use->parent_if, new_src);
- else
- nir_instr_rewrite_src(use->parent_instr, use, new_src);
-}
-
static bool
-evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t
*value)
+evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value)
{
nir_block *use_block = nir_cursor_current_block(cursor);
if (nir_block_dominates(nir_if_first_then_block(nif), use_block))
{
- *value = NIR_TRUE;
+ *value = true;
return true;
} else if (nir_block_dominates(nir_if_first_else_block(nif),
use_block)) {
- *value = NIR_FALSE;
+ *value = false;
return true;
} else {
return false;
@@ -460,52 +444,31 @@ propagate_condition_eval(nir_builder *b, nir_if
*nif, nir_src *use_src,
nir_src *alu_use, nir_alu_instr *alu,
bool is_if_condition)
{
- bool progress = false;
+ bool bool_value;
+ if (!evaluate_if_condition(nif, b->cursor, &bool_value))
+ return false;
- nir_const_value bool_value;
b->cursor = nir_before_src(alu_use, is_if_condition);
- if (nir_op_infos[alu->op].num_inputs == 1) {
- assert(alu->op == nir_op_inot || alu->op == nir_op_b2i);
-
- if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0]))
{
- assert(nir_src_bit_size(alu->src[0].src) == 32);
-
- nir_const_value result =
-nir_eval_const_opcode(alu->op, 1, 32, &bool_value);
- replace_if_condition_use_with_const(b, alu_use, result,
- is_if_condition);
- progress = true;
+ nir_ssa_def *def[2] = { };
+ for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
+ if (alu->src[i].src.ssa == use_src->ssa) {
+ def[i] = nir_imm_bool(b, bool_value);
+ } else {
+ def[i] = alu->src[i].src.ssa;
}
- } else {
- assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
-
- if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0]))
{
- nir_ssa_def *def[2];
- for (unsigned i = 0; i < 2; i++) {
-if (alu->src[i].src.ssa == use_src->ssa) {
- def[i] = nir_build_imm(b, 1, 32, bool_value);
-} else {
- def[i] = alu->src[i].src.ssa;
-}
- }
-
- nir_ssa_def *nalu =
-nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
-
- /* Rewrite use to use new alu instruction */
- nir_src new_src = nir_src_for_ssa(nalu);
+ }
+ nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1],
NULL, NULL);
- if (is_if_condition)
-nir_if_rewrite_condition(alu_use->parent_if, new_src);
- else
-nir_instr_rewrite_src(alu_use->parent_instr, alu_use,
new_src);
+ /* Rewrite use to use new alu instruction */
+ nir_src new_src = nir_src_for_ssa(nalu);
- progress = true;
- }
- }
+ if (is_if_condition)
+ nir_if_rewrite_condition(alu_use->parent_if, new_src);
+ else
+ nir_instr_rewrite_src(alu_use->parent_instr, alu_use,
new_src);
- return progress;
+ return true;
}
static bool
@@ -527,11 +490,17 @@ evaluate_condition_use(nir_builder *b, nir_if
*nif, nir_src *use_src,
{
bool progress = false;
- nir_const_value value;
b->cursor = nir_before_src(use_src, is_if_condition);
- if (evaluate_if_condition(nif, b->cursor, &value.u32[
Re: [Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation
Jason, JFYI, I have been looking into the cases where the boolean
bitsize lowering pass was producing worse instruction counts that the
default 32-bit pass and I have tracked it down to this patch. Reverting
this makes the instruction count much better for some tests, I'll check
why this happens tomorrow.
Iago
On Mon, 2018-10-22 at 17:13 -0500, Jason Ekstrand wrote:
> Instead of doing our own constant folding, we just emit instructions
> and
> let constant folding happen. This is substantially simpler and lets
> us
> use the nir_imm_bool helper instead of dealing with the const_value's
> ourselves.
> ---
> src/compiler/nir/nir_opt_if.c | 91 -
> --
> 1 file changed, 30 insertions(+), 61 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_if.c
> b/src/compiler/nir/nir_opt_if.c
> index 0c94aa170b5..60368a0259e 100644
> --- a/src/compiler/nir/nir_opt_if.c
> +++ b/src/compiler/nir/nir_opt_if.c
> @@ -377,31 +377,15 @@ opt_if_loop_terminator(nir_if *nif)
> return true;
> }
>
> -static void
> -replace_if_condition_use_with_const(nir_builder *b, nir_src *use,
> -nir_const_value nir_boolean,
> -bool if_condition)
> -{
> - /* Create const */
> - nir_ssa_def *const_def = nir_build_imm(b, 1, 32, nir_boolean);
> -
> - /* Rewrite use to use const */
> - nir_src new_src = nir_src_for_ssa(const_def);
> - if (if_condition)
> - nir_if_rewrite_condition(use->parent_if, new_src);
> - else
> - nir_instr_rewrite_src(use->parent_instr, use, new_src);
> -}
> -
> static bool
> -evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t
> *value)
> +evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value)
> {
> nir_block *use_block = nir_cursor_current_block(cursor);
> if (nir_block_dominates(nir_if_first_then_block(nif), use_block))
> {
> - *value = NIR_TRUE;
> + *value = true;
>return true;
> } else if (nir_block_dominates(nir_if_first_else_block(nif),
> use_block)) {
> - *value = NIR_FALSE;
> + *value = false;
>return true;
> } else {
>return false;
> @@ -460,52 +444,31 @@ propagate_condition_eval(nir_builder *b, nir_if
> *nif, nir_src *use_src,
> nir_src *alu_use, nir_alu_instr *alu,
> bool is_if_condition)
> {
> - bool progress = false;
> + bool bool_value;
> + if (!evaluate_if_condition(nif, b->cursor, &bool_value))
> + return false;
>
> - nir_const_value bool_value;
> b->cursor = nir_before_src(alu_use, is_if_condition);
> - if (nir_op_infos[alu->op].num_inputs == 1) {
> - assert(alu->op == nir_op_inot || alu->op == nir_op_b2i);
> -
> - if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0]))
> {
> - assert(nir_src_bit_size(alu->src[0].src) == 32);
> -
> - nir_const_value result =
> -nir_eval_const_opcode(alu->op, 1, 32, &bool_value);
>
> - replace_if_condition_use_with_const(b, alu_use, result,
> - is_if_condition);
> - progress = true;
> + nir_ssa_def *def[2] = { };
> + for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
> + if (alu->src[i].src.ssa == use_src->ssa) {
> + def[i] = nir_imm_bool(b, bool_value);
> + } else {
> + def[i] = alu->src[i].src.ssa;
>}
> - } else {
> - assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
> -
> - if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0]))
> {
> - nir_ssa_def *def[2];
> - for (unsigned i = 0; i < 2; i++) {
> -if (alu->src[i].src.ssa == use_src->ssa) {
> - def[i] = nir_build_imm(b, 1, 32, bool_value);
> -} else {
> - def[i] = alu->src[i].src.ssa;
> -}
> - }
> -
> - nir_ssa_def *nalu =
> -nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
> -
> - /* Rewrite use to use new alu instruction */
> - nir_src new_src = nir_src_for_ssa(nalu);
> + }
> + nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1],
> NULL, NULL);
>
> - if (is_if_condition)
> -nir_if_rewrite_condition(alu_use->parent_if, new_src);
> - else
> -nir_instr_rewrite_src(alu_use->parent_instr, alu_use,
> new_src);
> + /* Rewrite use to use new alu instruction */
> + nir_src new_src = nir_src_for_ssa(nalu);
>
> - progress = true;
> - }
> - }
> + if (is_if_condition)
> + nir_if_rewrite_condition(alu_use->parent_if, new_src);
> + else
> + nir_instr_rewrite_src(alu_use->parent_instr, alu_use,
> new_src);
>
> - return progress;
> + return true;
> }
>
> static bool
> @@ -527,11 +490,17 @@ evaluate_condition_use(nir_builder *b, nir_if
> *nif, nir_src *use_src,
> {
> bool progress = false;
>
> - nir_const_value value;
> b->curs
Re: [Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation
On 22/10/18 23:13, Jason Ekstrand wrote:
> Instead of doing our own constant folding, we just emit instructions and
> let constant folding happen. This is substantially simpler and lets us
> use the nir_imm_bool helper instead of dealing with the const_value's
> ourselves.
> ---
> src/compiler/nir/nir_opt_if.c | 91 ---
> 1 file changed, 30 insertions(+), 61 deletions(-)
>
> diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
> index 0c94aa170b5..60368a0259e 100644
> --- a/src/compiler/nir/nir_opt_if.c
> +++ b/src/compiler/nir/nir_opt_if.c
[...]
> + nir_ssa_def *def[2] = { };
I'm afraid empty struct/array initializers aren't standard C and MSVC
throws an error.
I really wish there was some GCC warning we could enable for this, as it
appears to be a common mistake, particularly on nir, probably due to all
simple POD structs. But I couldn't find one.
Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation
Looks ok to me.
Reviewed-by: Timothy Arceri
On 23/10/18 9:13 am, Jason Ekstrand wrote:
Instead of doing our own constant folding, we just emit instructions and
let constant folding happen. This is substantially simpler and lets us
use the nir_imm_bool helper instead of dealing with the const_value's
ourselves.
---
src/compiler/nir/nir_opt_if.c | 91 ---
1 file changed, 30 insertions(+), 61 deletions(-)
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index 0c94aa170b5..60368a0259e 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -377,31 +377,15 @@ opt_if_loop_terminator(nir_if *nif)
return true;
}
-static void
-replace_if_condition_use_with_const(nir_builder *b, nir_src *use,
-nir_const_value nir_boolean,
-bool if_condition)
-{
- /* Create const */
- nir_ssa_def *const_def = nir_build_imm(b, 1, 32, nir_boolean);
-
- /* Rewrite use to use const */
- nir_src new_src = nir_src_for_ssa(const_def);
- if (if_condition)
- nir_if_rewrite_condition(use->parent_if, new_src);
- else
- nir_instr_rewrite_src(use->parent_instr, use, new_src);
-}
-
static bool
-evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t *value)
+evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value)
{
nir_block *use_block = nir_cursor_current_block(cursor);
if (nir_block_dominates(nir_if_first_then_block(nif), use_block)) {
- *value = NIR_TRUE;
+ *value = true;
return true;
} else if (nir_block_dominates(nir_if_first_else_block(nif), use_block)) {
- *value = NIR_FALSE;
+ *value = false;
return true;
} else {
return false;
@@ -460,52 +444,31 @@ propagate_condition_eval(nir_builder *b, nir_if *nif,
nir_src *use_src,
nir_src *alu_use, nir_alu_instr *alu,
bool is_if_condition)
{
- bool progress = false;
+ bool bool_value;
+ if (!evaluate_if_condition(nif, b->cursor, &bool_value))
+ return false;
- nir_const_value bool_value;
b->cursor = nir_before_src(alu_use, is_if_condition);
- if (nir_op_infos[alu->op].num_inputs == 1) {
- assert(alu->op == nir_op_inot || alu->op == nir_op_b2i);
-
- if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0])) {
- assert(nir_src_bit_size(alu->src[0].src) == 32);
-
- nir_const_value result =
-nir_eval_const_opcode(alu->op, 1, 32, &bool_value);
- replace_if_condition_use_with_const(b, alu_use, result,
- is_if_condition);
- progress = true;
+ nir_ssa_def *def[2] = { };
+ for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
+ if (alu->src[i].src.ssa == use_src->ssa) {
+ def[i] = nir_imm_bool(b, bool_value);
+ } else {
+ def[i] = alu->src[i].src.ssa;
}
- } else {
- assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
-
- if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0])) {
- nir_ssa_def *def[2];
- for (unsigned i = 0; i < 2; i++) {
-if (alu->src[i].src.ssa == use_src->ssa) {
- def[i] = nir_build_imm(b, 1, 32, bool_value);
-} else {
- def[i] = alu->src[i].src.ssa;
-}
- }
-
- nir_ssa_def *nalu =
-nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
-
- /* Rewrite use to use new alu instruction */
- nir_src new_src = nir_src_for_ssa(nalu);
+ }
+ nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
- if (is_if_condition)
-nir_if_rewrite_condition(alu_use->parent_if, new_src);
- else
-nir_instr_rewrite_src(alu_use->parent_instr, alu_use, new_src);
+ /* Rewrite use to use new alu instruction */
+ nir_src new_src = nir_src_for_ssa(nalu);
- progress = true;
- }
- }
+ if (is_if_condition)
+ nir_if_rewrite_condition(alu_use->parent_if, new_src);
+ else
+ nir_instr_rewrite_src(alu_use->parent_instr, alu_use, new_src);
- return progress;
+ return true;
}
static bool
@@ -527,11 +490,17 @@ evaluate_condition_use(nir_builder *b, nir_if *nif,
nir_src *use_src,
{
bool progress = false;
- nir_const_value value;
b->cursor = nir_before_src(use_src, is_if_condition);
- if (evaluate_if_condition(nif, b->cursor, &value.u32[0])) {
- replace_if_condition_use_with_const(b, use_src, value, is_if_condition);
+ bool bool_value;
+ if (evaluate_if_condition(nif, b->cursor, &bool_value)) {
+ /* Rewrite use to use const */
+ nir_src imm_src = nir_src_for_ssa(nir_imm_bool(b, bool_value));
+ if (is_if_condition)
+ nir_if_rewrite_condition(use_src->parent_if, imm_src);
+ else
+ nir_instr_rew
[Mesa-dev] [PATCH 07/31] nir/opt_if: Rework condition propagation
Instead of doing our own constant folding, we just emit instructions and
let constant folding happen. This is substantially simpler and lets us
use the nir_imm_bool helper instead of dealing with the const_value's
ourselves.
---
src/compiler/nir/nir_opt_if.c | 91 ---
1 file changed, 30 insertions(+), 61 deletions(-)
diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c
index 0c94aa170b5..60368a0259e 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -377,31 +377,15 @@ opt_if_loop_terminator(nir_if *nif)
return true;
}
-static void
-replace_if_condition_use_with_const(nir_builder *b, nir_src *use,
-nir_const_value nir_boolean,
-bool if_condition)
-{
- /* Create const */
- nir_ssa_def *const_def = nir_build_imm(b, 1, 32, nir_boolean);
-
- /* Rewrite use to use const */
- nir_src new_src = nir_src_for_ssa(const_def);
- if (if_condition)
- nir_if_rewrite_condition(use->parent_if, new_src);
- else
- nir_instr_rewrite_src(use->parent_instr, use, new_src);
-}
-
static bool
-evaluate_if_condition(nir_if *nif, nir_cursor cursor, uint32_t *value)
+evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value)
{
nir_block *use_block = nir_cursor_current_block(cursor);
if (nir_block_dominates(nir_if_first_then_block(nif), use_block)) {
- *value = NIR_TRUE;
+ *value = true;
return true;
} else if (nir_block_dominates(nir_if_first_else_block(nif), use_block)) {
- *value = NIR_FALSE;
+ *value = false;
return true;
} else {
return false;
@@ -460,52 +444,31 @@ propagate_condition_eval(nir_builder *b, nir_if *nif,
nir_src *use_src,
nir_src *alu_use, nir_alu_instr *alu,
bool is_if_condition)
{
- bool progress = false;
+ bool bool_value;
+ if (!evaluate_if_condition(nif, b->cursor, &bool_value))
+ return false;
- nir_const_value bool_value;
b->cursor = nir_before_src(alu_use, is_if_condition);
- if (nir_op_infos[alu->op].num_inputs == 1) {
- assert(alu->op == nir_op_inot || alu->op == nir_op_b2i);
-
- if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0])) {
- assert(nir_src_bit_size(alu->src[0].src) == 32);
-
- nir_const_value result =
-nir_eval_const_opcode(alu->op, 1, 32, &bool_value);
- replace_if_condition_use_with_const(b, alu_use, result,
- is_if_condition);
- progress = true;
+ nir_ssa_def *def[2] = { };
+ for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
+ if (alu->src[i].src.ssa == use_src->ssa) {
+ def[i] = nir_imm_bool(b, bool_value);
+ } else {
+ def[i] = alu->src[i].src.ssa;
}
- } else {
- assert(alu->op == nir_op_ior || alu->op == nir_op_iand);
-
- if (evaluate_if_condition(nif, b->cursor, &bool_value.u32[0])) {
- nir_ssa_def *def[2];
- for (unsigned i = 0; i < 2; i++) {
-if (alu->src[i].src.ssa == use_src->ssa) {
- def[i] = nir_build_imm(b, 1, 32, bool_value);
-} else {
- def[i] = alu->src[i].src.ssa;
-}
- }
-
- nir_ssa_def *nalu =
-nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
-
- /* Rewrite use to use new alu instruction */
- nir_src new_src = nir_src_for_ssa(nalu);
+ }
+ nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], NULL, NULL);
- if (is_if_condition)
-nir_if_rewrite_condition(alu_use->parent_if, new_src);
- else
-nir_instr_rewrite_src(alu_use->parent_instr, alu_use, new_src);
+ /* Rewrite use to use new alu instruction */
+ nir_src new_src = nir_src_for_ssa(nalu);
- progress = true;
- }
- }
+ if (is_if_condition)
+ nir_if_rewrite_condition(alu_use->parent_if, new_src);
+ else
+ nir_instr_rewrite_src(alu_use->parent_instr, alu_use, new_src);
- return progress;
+ return true;
}
static bool
@@ -527,11 +490,17 @@ evaluate_condition_use(nir_builder *b, nir_if *nif,
nir_src *use_src,
{
bool progress = false;
- nir_const_value value;
b->cursor = nir_before_src(use_src, is_if_condition);
- if (evaluate_if_condition(nif, b->cursor, &value.u32[0])) {
- replace_if_condition_use_with_const(b, use_src, value, is_if_condition);
+ bool bool_value;
+ if (evaluate_if_condition(nif, b->cursor, &bool_value)) {
+ /* Rewrite use to use const */
+ nir_src imm_src = nir_src_for_ssa(nir_imm_bool(b, bool_value));
+ if (is_if_condition)
+ nir_if_rewrite_condition(use_src->parent_if, imm_src);
+ else
+ nir_instr_rewrite_src(use_src->parent_instr, use_src, imm_src);
+
progress = true;
}
--
2.19.1
