[Mesa-dev] [PATCH] nir/search: Save/restore the variables_seen bitmask when matching

[Ian Romanick]

On 05/08/2015 11:55 AM, Jason Ekstrand wrote:
> On Fri, May 8, 2015 at 11:53 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> total instructions in shared programs: 7152330 -> 7137006 (-0.21%)
>> instructions in affected programs:     1330548 -> 1315224 (-1.15%)
>> helped:                                5797
>> HURT:                                  76
> 
> I'm doing some looking into the hurt programs.  It seems as if there
> are some very strange interatctions between flrp and ffma.  I'm still
> working out exactly how to fix it up.

Yes, I noticed this too.  Did you see my reply to Ken earlier today?
The problem I noticed /seems/ restricted to cases where the would-be
interpolant is scalar but the other values are vector.

> --Jason
> 
>> GAINED:                                0
>> LOST:                                  8
>> ---
>>  src/glsl/nir/nir_search.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/src/glsl/nir/nir_search.c b/src/glsl/nir/nir_search.c
>> index d86655b..4c83349 100644
>> --- a/src/glsl/nir/nir_search.c
>> +++ b/src/glsl/nir/nir_search.c
>> @@ -199,6 +199,10 @@ match_expression(const nir_search_expression *expr, nir_alu_instr *instr,
>>        }
>>     }
>>
>> +   /* Stash off the current variables_seen bitmask.  This way we can
>> +    * restore it prior to matching in the commutative case below. */
>> +   unsigned variables_seen_stash = state->variables_seen;
>> +
>>     bool matched = true;
>>     for (unsigned i = 0; i < nir_op_infos[instr->op].num_inputs; i++) {
>>        /* If the source is an explicitly sized source, then we need to reset
>> @@ -221,6 +225,13 @@ match_expression(const nir_search_expression *expr, nir_alu_instr *instr,
>>
>>     if (nir_op_infos[instr->op].algebraic_properties & NIR_OP_IS_COMMUTATIVE) {
>>        assert(nir_op_infos[instr->op].num_inputs == 2);
>> +
>> +      /* Restore the variables_seen bitmask.  If we don't do this, then we
>> +       * could end up with an erroneous failure due to variables found in the
>> +       * first match attempt above not matching those in the second.
>> +       */
>> +      state->variables_seen = variables_seen_stash;
>> +
>>        if (!match_value(expr->srcs[0], instr, 1, num_components,
>>                         swizzle, state))
>>           return false;
>> --
>> 2.4.0
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>