From 035f66025b4d82ce40573da86bde6afda4a05ec7 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Mon, 28 Mar 2016 11:12:33 -0700 Subject: [PATCH] nir/search: Don't match inexact expressions with exact subexpressions In the first pass of implementing exact handling, I made a mistake with search-and-replace. In particular, we only reallly handled exact/inexact on the root of the tree. Instead, we need to check every node in the tree for an exact/inexact match. As an example of this, consider the following GLSL code precise float a = b + c; if (a < 0) { do_stuff(); } In that case, only the add will be declared "exact" and an expression that looks for "b + c < 0" will still match and replace it with "b < -c" which may yield different results. The solution is to simply bail if any of the values are exact when matching an inexact expression. --- src/compiler/nir/nir_search.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/compiler/nir/nir_search.c b/src/compiler/nir/nir_search.c index 6e630631453..b915101ce32 100644 --- a/src/compiler/nir/nir_search.c +++ b/src/compiler/nir/nir_search.c @@ -28,6 +28,8 @@ #include "nir_search.h" struct match_state { + bool inexact_match; + bool has_exact_alu; unsigned variables_seen; nir_alu_src variables[NIR_SEARCH_MAX_VARIABLES]; }; @@ -239,7 +241,10 @@ match_expression(const nir_search_expression *expr, nir_alu_instr *instr, return false; assert(instr->dest.dest.is_ssa); - if (expr->inexact && instr->exact) + + state->inexact_match = expr->inexact || state->inexact_match; + state->has_exact_alu = instr->exact || state->has_exact_alu; + if (state->inexact_match && state->has_exact_alu) return false; assert(!instr->dest.saturate); @@ -410,7 +415,7 @@ bitsize_tree_filter_down(bitsize_tree *tree, unsigned size) static nir_alu_src construct_value(const nir_search_value *value, - unsigned num_components, bitsize_tree *bitsize, bool exact, + unsigned num_components, bitsize_tree *bitsize, struct match_state *state, nir_instr *instr, void *mem_ctx) { @@ -424,10 +429,16 @@ construct_value(const nir_search_value *value, nir_alu_instr *alu = nir_alu_instr_create(mem_ctx, expr->opcode); nir_ssa_dest_init(&alu->instr, &alu->dest.dest, num_components, bitsize->dest_size, NULL); - alu->exact = exact; alu->dest.write_mask = (1 << num_components) - 1; alu->dest.saturate = false; + /* We have no way of knowing what values in a given search expression + * map to a particular replacement value. Therefore, if the + * expression we are replacing has any exact values, the entire + * replacement should be exact. + */ + alu->exact = state->has_exact_alu; + for (unsigned i = 0; i < nir_op_infos[expr->opcode].num_inputs; i++) { /* If the source is an explicitly sized source, then we need to reset * the number of components to match. @@ -436,7 +447,7 @@ construct_value(const nir_search_value *value, num_components = nir_op_infos[alu->op].input_sizes[i]; alu->src[i] = construct_value(expr->srcs[i], - num_components, bitsize->srcs[i], exact, + num_components, bitsize->srcs[i], state, instr, mem_ctx); } @@ -546,6 +557,8 @@ nir_replace_instr(nir_alu_instr *instr, const nir_search_expression *search, assert(instr->dest.dest.is_ssa); struct match_state state; + state.inexact_match = false; + state.has_exact_alu = false; state.variables_seen = 0; if (!match_expression(search, instr, instr->dest.dest.ssa.num_components, @@ -569,7 +582,7 @@ nir_replace_instr(nir_alu_instr *instr, const nir_search_expression *search, mov->src[0] = construct_value(replace, instr->dest.dest.ssa.num_components, tree, - instr->exact, &state, &instr->instr, mem_ctx); + &state, &instr->instr, mem_ctx); nir_instr_insert_before(&instr->instr, &mov->instr); nir_ssa_def_rewrite_uses(&instr->dest.dest.ssa,