On Tue, Mar 29, 2016 at 2:50 PM, Ian Romanick <[email protected]> wrote:
> Does this fix any test cases? Either way, this series is > No know test cases. It does mean that we now correctly implement the semantics required for SPIR-V > Reviewed-by: Ian Romanick <[email protected]> > > On 03/28/2016 04:54 PM, Jason Ekstrand wrote: > > 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 6e63063..b915101 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, > > > >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
