[Mesa-dev] Mesa (master): nir/flrp: Lower flrp(±1, b, c) and flrp(a, ±1, c) differently

Samuel Pitoiset samuel.pitoiset at gmail.com
Tue May 7 15:35:24 UTC 2019


On 5/7/19 5:30 PM, Ian Romanick wrote:
> On 5/7/19 8:20 AM, Samuel Pitoiset wrote:
>> This introduces glitches with Talos and Serious Sam 2017 with RADV...
>>
>> Are you able to reproduce the problem with ANV?
> Probably not very easily.  If you can figure out which shader it is, it
> should be easy to figure out the problem from before / after NIR.
The NIR before/after is *very* different, not easy to find the 
differences actually.
>
>> On 5/7/19 8:01 AM, GitLab Mirror wrote:
>>> Module: Mesa
>>> Branch: master
>>> Commit: 5b908db604b2f47bb8382047533e556db8d5f52b
>>> URL:
>>> http://cgit.freedesktop.org/mesa/mesa/commit/?id=5b908db604b2f47bb8382047533e556db8d5f52b
>>>
>>>
>>> Author: Ian Romanick <ian.d.romanick at intel.com>
>>> Date:   Tue Aug 21 17:17:24 2018 -0700
>>>
>>> nir/flrp: Lower flrp(±1, b, c) and flrp(a, ±1, c) differently
>>>
>>> No changes on any other Intel platforms.
>>>
>>> v2: Rebase on 424372e5dd5 ("nir: Use the flrp lowering pass instead of
>>> nir_opt_algebraic")
>>>
>>> Iron Lake and GM45 had similar results. (Iron Lake shown)
>>> total instructions in shared programs: 8189888 -> 8153912 (-0.44%)
>>> instructions in affected programs: 1199037 -> 1163061 (-3.00%)
>>> helped: 4124
>>> HURT: 10
>>> helped stats (abs) min: 1 max: 40 x̄: 8.73 x̃: 9
>>> helped stats (rel) min: 0.20% max: 86.96% x̄: 4.96% x̃: 3.02%
>>> HURT stats (abs)   min: 1 max: 2 x̄: 1.20 x̃: 1
>>> HURT stats (rel)   min: 1.06% max: 3.92% x̄: 1.62% x̃: 1.06%
>>> 95% mean confidence interval for instructions value: -8.84 -8.56
>>> 95% mean confidence interval for instructions %-change: -5.12% -4.77%
>>> Instructions are helped.
>>>
>>> total cycles in shared programs: 188606710 -> 188426964 (-0.10%)
>>> cycles in affected programs: 27505596 -> 27325850 (-0.65%)
>>> helped: 4026
>>> HURT: 77
>>> helped stats (abs) min: 2 max: 646 x̄: 44.99 x̃: 46
>>> helped stats (rel) min: <.01% max: 94.58% x̄: 2.35% x̃: 0.85%
>>> HURT stats (abs)   min: 2 max: 376 x̄: 17.79 x̃: 6
>>> HURT stats (rel)   min: <.01% max: 2.60% x̄: 0.22% x̃: 0.04%
>>> 95% mean confidence interval for cycles value: -44.75 -42.87
>>> 95% mean confidence interval for cycles %-change: -2.44% -2.17%
>>> Cycles are helped.
>>>
>>> LOST:   3
>>> GAINED: 35
>>>
>>> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>>>
>>> ---
>>>
>>>    src/compiler/nir/nir_lower_flrp.c | 134
>>> ++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 134 insertions(+)
>>>
>>> diff --git a/src/compiler/nir/nir_lower_flrp.c
>>> b/src/compiler/nir/nir_lower_flrp.c
>>> index 952068ec9cc..c041fefc52b 100644
>>> --- a/src/compiler/nir/nir_lower_flrp.c
>>> +++ b/src/compiler/nir/nir_lower_flrp.c
>>> @@ -137,6 +137,89 @@ replace_with_fast(struct nir_builder *bld, struct
>>> u_vector *dead_flrp,
>>>       append_flrp_to_dead_list(dead_flrp, alu);
>>>    }
>>>    +/**
>>> + * Replace flrp(a, b, c) with (b*c ± c) + a
>>> + */
>>> +static void
>>> +replace_with_expanded_ffma_and_add(struct nir_builder *bld,
>>> +                                   struct u_vector *dead_flrp,
>>> +                                   struct nir_alu_instr *alu, bool
>>> subtract_c)
>>> +{
>>> +   nir_ssa_def *const a = nir_ssa_for_alu_src(bld, alu, 0);
>>> +   nir_ssa_def *const b = nir_ssa_for_alu_src(bld, alu, 1);
>>> +   nir_ssa_def *const c = nir_ssa_for_alu_src(bld, alu, 2);
>>> +
>>> +   nir_ssa_def *const b_times_c = nir_fadd(bld, b, c);
>>> +   nir_instr_as_alu(b_times_c->parent_instr)->exact = alu->exact;
>>> +
>>> +   nir_ssa_def *inner_sum;
>>> +
>>> +   if (subtract_c) {
>>> +      nir_ssa_def *const neg_c = nir_fneg(bld, c);
>>> +      nir_instr_as_alu(neg_c->parent_instr)->exact = alu->exact;
>>> +
>>> +      inner_sum = nir_fadd(bld, b_times_c, neg_c);
>>> +   } else {
>>> +      inner_sum = nir_fadd(bld, b_times_c, c);
>>> +   }
>>> +
>>> +   nir_instr_as_alu(inner_sum->parent_instr)->exact = alu->exact;
>>> +
>>> +   nir_ssa_def *const outer_sum = nir_fadd(bld, inner_sum, a);
>>> +   nir_instr_as_alu(outer_sum->parent_instr)->exact = alu->exact;
>>> +
>>> +   nir_ssa_def_rewrite_uses(&alu->dest.dest.ssa,
>>> nir_src_for_ssa(outer_sum));
>>> +
>>> +   /* DO NOT REMOVE the original flrp yet.  Many of the lowering
>>> choices are
>>> +    * based on other uses of the sources.  Removing the flrp may
>>> cause the
>>> +    * last flrp in a sequence to make a different, incorrect choice.
>>> +    */
>>> +   append_flrp_to_dead_list(dead_flrp, alu);
>>> +}
>>> +
>>> +/**
>>> + * Determines whether a swizzled source is constant w/ all components
>>> the same.
>>> + *
>>> + * The value of the constant is stored in \c result.
>>> + *
>>> + * \return
>>> + * True if all components of the swizzled source are the same constant.
>>> + * Otherwise false is returned.
>>> + */
>>> +static bool
>>> +all_same_constant(const nir_alu_instr *instr, unsigned src, double
>>> *result)
>>> +{
>>> +   nir_const_value *val = nir_src_as_const_value(instr->src[src].src);
>>> +
>>> +   if (!val)
>>> +      return false;
>>> +
>>> +   const uint8_t *const swizzle = instr->src[src].swizzle;
>>> +   const unsigned num_components =
>>> nir_dest_num_components(instr->dest.dest);
>>> +
>>> +   if (instr->dest.dest.ssa.bit_size == 32) {
>>> +      const float first = val[swizzle[0]].f32;
>>> +
>>> +      for (unsigned i = 1; i < num_components; i++) {
>>> +         if (val[swizzle[i]].f32 != first)
>>> +            return false;
>>> +      }
>>> +
>>> +      *result = first;
>>> +   } else {
>>> +      const double first = val[swizzle[0]].f64;
>>> +
>>> +      for (unsigned i = 1; i < num_components; i++) {
>>> +         if (val[swizzle[i]].f64 != first)
>>> +            return false;
>>> +      }
>>> +
>>> +      *result = first;
>>> +   }
>>> +
>>> +   return true;
>>> +}
>>> +
>>>    static bool
>>>    sources_are_constants_with_similar_magnitudes(const nir_alu_instr
>>> *instr)
>>>    {
>>> @@ -265,6 +348,57 @@ convert_flrp_instruction(nir_builder *bld,
>>>          return;
>>>       }
>>>    +   /*
>>> +    * - If x = 1:
>>> +    *
>>> +    *        (yt + -t) + 1
>>> +    *
>>> +    * - If x = -1:
>>> +    *
>>> +    *        (yt + t) - 1
>>> +    *
>>> +    *   In both cases, x is used in place of ±1 for simplicity.  Both
>>> forms
>>> +    *   lend to ffma generation on platforms that support ffma.
>>> +    */
>>> +   double src0_as_constant;
>>> +   if (all_same_constant(alu, 0, &src0_as_constant)) {
>>> +      if (src0_as_constant == 1.0) {
>>> +         replace_with_expanded_ffma_and_add(bld, dead_flrp, alu,
>>> +                                            true /* subtract t */);
>>> +         return;
>>> +      } else if (src0_as_constant == -1.0) {
>>> +         replace_with_expanded_ffma_and_add(bld, dead_flrp, alu,
>>> +                                            false /* add t */);
>>> +         return;
>>> +      }
>>> +   }
>>> +
>>> +   /*
>>> +    * - If y = ±1:
>>> +    *
>>> +    *        x(1 - t) + yt
>>> +    *
>>> +    *   In this case either the multiply in yt will be eliminated by
>>> +    *   nir_opt_algebraic.  If FMA is supported, this results in
>>> fma(x, (1 -
>>> +    *   t), ±t) for two instructions.  If FMA is not supported, then
>>> the cost
>>> +    *   is 3 instructions.  We rely on nir_opt_algebraic to generate
>>> the FMA
>>> +    *   instructions as well.
>>> +    *
>>> +    *   Another possible replacement is
>>> +    *
>>> +    *        -xt + x ± t
>>> +    *
>>> +    *   Some groupings of this may be better on some platforms in some
>>> +    *   circumstances, bit it is probably dependent on scheduling.
>>> Futher
>>> +    *   investigation may be required.
>>> +    */
>>> +   double src1_as_constant;
>>> +   if ((all_same_constant(alu, 1, &src1_as_constant) &&
>>> +        (src1_as_constant == -1.0 || src1_as_constant == 1.0))) {
>>> +      replace_with_strict(bld, dead_flrp, alu);
>>> +      return;
>>> +   }
>>> +
>>>       if (have_ffma) {
>>>          if (always_precise) {
>>>             replace_with_strict_ffma(bld, dead_flrp, alu);
>>>
>>> _______________________________________________
>>> mesa-commit mailing list
>>> mesa-commit at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-commit
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list