On 1/11/18 1:28 am, Jason Ekstrand wrote:
On Tue, Oct 30, 2018 at 9:17 PM Timothy Arceri <[email protected]
<mailto:[email protected]>> wrote:
With the simplifications to this pass in a3b4cb34589e2f1a68 we
can allow any alu instruction to be processed. For one this can
potentially help with bcsels.
Do we want to? I believe that this patch is correct and I agree that we
now can but then we get into some heuristics as to when it is or isn't
useful. With iand, ior, and inot it's obviously useful because, for
boolean values, those instructions can be sort of half-folded, i.e. we
know that opt_algebraic will look at them and replace them with either
the other source or a constant. With bcsel that's true if the condition
is used in src[0] but not for src[1] or src[2]. For other instructions,
it's even less obvious that it helps.
I'm not saying I'm opposed. Just thinking out loud.
I'm not sure either. Ian (Ccing) was asking about support for bcsel when
I first wrote the series, I held off sending this patch in the past (due
to no shader-db change) but since I was looking at the code again I
thought I might as well send it out and get comments.
I'm happy to hold it back for now but I think we still have many
opportunities for improving bcsels so maybe it will be useful in future.
--Jason
No shader-db change.
---
src/compiler/nir/nir_opt_if.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/src/compiler/nir/nir_opt_if.c
b/src/compiler/nir/nir_opt_if.c
index 1fe95e53766..38489676e6b 100644
--- a/src/compiler/nir/nir_opt_if.c
+++ b/src/compiler/nir/nir_opt_if.c
@@ -448,7 +448,7 @@ propagate_condition_eval(nir_builder *b, nir_if
*nif, nir_src *use_src,
if (!evaluate_if_condition(nif, b->cursor, &bool_value))
return false;
- nir_ssa_def *def[2] = {0};
+ nir_ssa_def *def[4] = {0};
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);
@@ -456,7 +456,7 @@ propagate_condition_eval(nir_builder *b, nir_if
*nif, nir_src *use_src,
def[i] = alu->src[i].src.ssa;
}
}
- nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1],
NULL, NULL);
+ nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1],
def[2], def[3]);
/* Rewrite use to use new alu instruction */
nir_src new_src = nir_src_for_ssa(nalu);
@@ -469,19 +469,6 @@ propagate_condition_eval(nir_builder *b, nir_if
*nif, nir_src *use_src,
return true;
}
-static bool
-can_propagate_through_alu(nir_src *src)
-{
- if (src->parent_instr->type == nir_instr_type_alu &&
- (nir_instr_as_alu(src->parent_instr)->op == nir_op_ior ||
- nir_instr_as_alu(src->parent_instr)->op == nir_op_iand ||
- nir_instr_as_alu(src->parent_instr)->op == nir_op_inot ||
- nir_instr_as_alu(src->parent_instr)->op == nir_op_b2i))
- return true;
-
- return false;
-}
-
static bool
evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src,
bool is_if_condition)
@@ -502,7 +489,8 @@ evaluate_condition_use(nir_builder *b, nir_if
*nif, nir_src *use_src,
progress = true;
}
- if (!is_if_condition && can_propagate_through_alu(use_src)) {
+ if (!is_if_condition &&
+ use_src->parent_instr->type == nir_instr_type_alu) {
nir_alu_instr *alu = nir_instr_as_alu(use_src->parent_instr);
nir_foreach_use_safe(alu_use, &alu->dest.dest.ssa) {
--
2.17.2
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
