nir/lower_blend: Avoid emitting unnecessary fsats
authorAlyssa Rosenzweig <alyssa@rosenzweig.io>
Sat, 10 Sep 2022 20:08:13 +0000 (16:08 -0400)
committerMarge Bot <emma+marge@anholt.net>
Mon, 12 Sep 2022 23:44:54 +0000 (23:44 +0000)
The option struct passed to nir_lower_blend doesn't have a "blending
disabled" flag. Unless blending is skipped due to logic ops or
framebuffer formats, nir_lower_blend always blends, even if the blend
mode is "replace" (corresponding to the API level blend disable).

That's mostly okay, since NIR can optimize out the code, at the expense
of a little compile time. However, there's a catch: nir_lower_blend
emits fsat at the start of the shader (for UNORM framebuffers, or
fsat_signed for SNORM). We can expect hardware to saturate the input to
store_output itself, so these operations are redundant, but it's tricky
to optimize these instructions out otherwise. Don't even try: detect the
replace blend mode and don't call nir_blend in that case. Colour masking
is still applied as usual.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Emma Anholt <emma@anholt.net>
Reviewed-by: Jason Ekstrand <jason.ekstrand@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18535>

src/compiler/nir/nir_lower_blend.c

index b844d70..727d5d2 100644 (file)
@@ -365,6 +365,25 @@ color_index_for_var(const nir_variable *var)
           (var->data.location - FRAG_RESULT_DATA0);
 }
 
+/*
+ * Test if the blending options for a given channel encode the "replace" blend
+ * mode: dest = source. In this case, blending may be specially optimized.
+ */
+static bool
+nir_blend_replace_channel(const nir_lower_blend_channel *c)
+{
+   return (c->func == BLEND_FUNC_ADD) &&
+          (c->src_factor == BLEND_FACTOR_ZERO && c->invert_src_factor) &&
+          (c->dst_factor == BLEND_FACTOR_ZERO && !c->invert_dst_factor);
+}
+
+static bool
+nir_blend_replace_rt(const nir_lower_blend_rt *rt)
+{
+   return nir_blend_replace_channel(&rt->rgb) &&
+          nir_blend_replace_channel(&rt->alpha);
+}
+
 static bool
 nir_lower_blend_store(nir_builder *b, nir_intrinsic_instr *store,
                       const nir_lower_blend_options *options)
@@ -392,12 +411,18 @@ nir_lower_blend_store(nir_builder *b, nir_intrinsic_instr *store,
    b->shader->info.fs.uses_fbfetch_output = true;
    nir_ssa_def *dst = nir_pad_vector(b, nir_load_var(b, var), 4);
 
-   /* Blend the two colors per the passed options */
+   /* Blend the two colors per the passed options. We only call nir_blend if
+    * blending is enabled with a blend mode other than replace (independent of
+    * the color mask). That avoids unnecessary fsat instructions in the common
+    * case where blending is disabled at an API level, but the driver calls
+    * nir_blend (possibly for color masking).
+    */
    nir_ssa_def *blended = src;
 
    if (options->logicop_enable) {
       blended = nir_blend_logicop(b, options, rt, src, dst);
-   } else if (!util_format_is_pure_integer(options->format[rt])) {
+   } else if (!util_format_is_pure_integer(options->format[rt]) &&
+              !nir_blend_replace_rt(&options->rt[rt])) {
       assert(!util_format_is_scaled(options->format[rt]));
       blended = nir_blend(b, options, rt, src, options->src1, dst);
    }