[Mesa-dev] [PATCH 4/7] nir: Add nir_lower_blend pass
Ian Romanick
idr at freedesktop.org
Tue May 7 19:18:12 UTC 2019
On 5/5/19 7:26 PM, Alyssa Rosenzweig wrote:
> This new lowering pass implements the OpenGL ES blend pipeline in
> shaders, applicable to hardware lacking full-featured blending hardware
> (including Midgard/Bifrost and vc4). This pass is run on a fragment
> shader, rewriting the store to a blended version, loading in the
> framebuffer destination color and constant color via intrinsics as
> necessary. This pass is sufficient for OpenGL ES 2.0 and is verified to
> pass dEQP's blend tests. That said, at present it has the following
> limitations:
>
> - MRT is not supported.
> - Logic ops are not supported.
Logic ops seem... challenging to emulate in the shader. That shader
would need the destination colors in the framebuffer storage format, and
I'm not sure that's always possible (maybe?). We (and I think everyone
else) removed GL_EXT_blend_logic_op because nobody's hardware could
handle the interactions GL_EXT_blend_equation_separate. It would be
cool to add it back. :)
It might also be fun to add support for GL_AMD_blend_minmax_factor and
GL_SGIX_blend_alpha_minmax. Looking at this lowering pass, it seems
like most of the work would be adding tests.
> MRT support is on my TODO list but paused until MRT is implemented in
> the rest of the driver. Both changes should be fairly trivial.
>
> It also includes MIN/MAX modes, so in conjunction with the advanced
> blend mode lowering it should be sufficient for ES3, though this has not
> been thoroughly tested. It is an open question whether the current GLSL
> IR based advanced blend lowering should be NIRified and merged into this
> pass.
Having all of the lowering related to blending in one place seems like a
good idea.
> ...Dual-source blending is not supported, Ryan.
>
> Signed-off-by: Alyssa Rosenzweig <alyssa at rosenzweig.io>
> Cc: Eric Anholt <eric at anholt.net>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> ---
> src/compiler/Makefile.sources | 1 +
> src/compiler/nir/meson.build | 1 +
> src/compiler/nir/nir.h | 22 +++
> src/compiler/nir/nir_lower_blend.c | 214 +++++++++++++++++++++++++++++
> 4 files changed, 238 insertions(+)
> create mode 100644 src/compiler/nir/nir_lower_blend.c
>
> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
> index 9bebc3d8867..d68b9550b02 100644
> --- a/src/compiler/Makefile.sources
> +++ b/src/compiler/Makefile.sources
> @@ -238,6 +238,7 @@ NIR_FILES = \
> nir/nir_lower_bit_size.c \
> nir/nir_lower_bool_to_float.c \
> nir/nir_lower_bool_to_int32.c \
> + nir/nir_lower_blend.c \
> nir/nir_lower_clamp_color_outputs.c \
> nir/nir_lower_clip.c \
> nir/nir_lower_clip_cull_distance_arrays.c \
> diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build
> index a8faeb9c018..73ab62d4b46 100644
> --- a/src/compiler/nir/meson.build
> +++ b/src/compiler/nir/meson.build
> @@ -116,6 +116,7 @@ files_libnir = files(
> 'nir_lower_array_deref_of_vec.c',
> 'nir_lower_atomics_to_ssbo.c',
> 'nir_lower_bitmap.c',
> + 'nir_lower_blend.c',
> 'nir_lower_bool_to_float.c',
> 'nir_lower_bool_to_int32.c',
> 'nir_lower_clamp_color_outputs.c',
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 37161e83e4d..8b68faed819 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -3447,6 +3447,28 @@ typedef enum {
>
> bool nir_lower_to_source_mods(nir_shader *shader, nir_lower_to_source_mods_flags options);
>
> +/* These structs encapsulates the blend state such that it can be lowered
encapsulate
> + * cleanly */
*/ on its own line. There's at least one more instance of this below.
> +
> +typedef struct {
> + enum blend_func func;
> +
> + enum blend_factor src_factor;
> + bool invert_src_factor;
> +
> + enum blend_factor dst_factor;
> + bool invert_dst_factor;
> +} nir_lower_blend_channel;
> +
> +typedef struct {
> + struct {
> + nir_lower_blend_channel rgb;
> + nir_lower_blend_channel alpha;
> + } rt[8];
> +} nir_lower_blend_options;
> +
> +void nir_lower_blend(nir_shader *shader, nir_lower_blend_options options);
> +
> bool nir_lower_gs_intrinsics(nir_shader *shader);
>
> typedef unsigned (*nir_lower_bit_size_callback)(const nir_alu_instr *, void *);
> diff --git a/src/compiler/nir/nir_lower_blend.c b/src/compiler/nir/nir_lower_blend.c
> new file mode 100644
> index 00000000000..5a874f08834
> --- /dev/null
> +++ b/src/compiler/nir/nir_lower_blend.c
> @@ -0,0 +1,214 @@
> +/*
> + * Copyright (C) 2019 Alyssa Rosenzweig
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +/**
> + * @file
> + *
> + * Implements the fragment pipeline (blending and writeout) in software, to be
> + * run as a dedicated "blend shader" stage on Midgard/Bifrost, or as a fragment
> + * shader variant on typical GPUs. This pass is useful if hardware lacks
> + * fixed-function blending in part or in full.
> + */
> +
> +#include "nir/nir.h"
> +#include "nir/nir_builder.h"
> +
> +/* Given processed factors, combine them per a blend function */
> +
> +static nir_ssa_def *
> +nir_blend_func(
> + nir_builder *b,
> + enum blend_func func,
> + nir_ssa_def *src, nir_ssa_def *dst)
> +{
> + switch (func) {
> + case BLEND_FUNC_ADD:
Alas, case should be at the same indentation level as switch.
> + return nir_fadd(b, src, dst);
> + case BLEND_FUNC_SUBTRACT:
> + return nir_fsub(b, src, dst);
> + case BLEND_FUNC_REVERSE_SUBTRACT:
> + return nir_fsub(b, dst, src);
> + case BLEND_FUNC_MIN:
> + return nir_fmin(b, src, dst);
> + case BLEND_FUNC_MAX:
> + return nir_fmax(b, src, dst);
> + default:
> + unreachable("Invalid blend function");
> + break;
Since factor is an enum, I think it's better to not have the default
case. If there's no default case, the compiler will issue a warning if
a new enum is added but not handled.
Either way, the break is definitely not necessary.
> + }
> +}
> +
> +/* Does this blend function multiply by a blend factor? */
> +
> +static bool
> +nir_blend_factored(enum blend_func func) {
{ goes on it's own line.
> + switch (func) {
> + case BLEND_FUNC_ADD:
> + case BLEND_FUNC_SUBTRACT:
> + case BLEND_FUNC_REVERSE_SUBTRACT:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +/* Returns a scalar single factor, unmultiplied */
> +
> +static nir_ssa_def *
> +nir_blend_factor_value(
> + nir_builder *b,
> + nir_ssa_def *src, nir_ssa_def *dst, nir_ssa_def *bconst,
> + unsigned chan,
> + enum blend_factor factor)
> +{
> + switch (factor) {
> + case BLEND_FACTOR_ZERO:
> + return nir_imm_float(b, 0.0);
> + case BLEND_FACTOR_SRC_COLOR:
> + return nir_channel(b, src, chan);
> + case BLEND_FACTOR_DST_COLOR:
> + return nir_channel(b, dst, chan);
> + case BLEND_FACTOR_SRC_ALPHA:
> + return nir_channel(b, src, 3);
> + case BLEND_FACTOR_DST_ALPHA:
> + return nir_channel(b, dst, 3);
> + case BLEND_FACTOR_CONSTANT_COLOR:
> + return nir_channel(b, bconst, chan);
> + case BLEND_FACTOR_CONSTANT_ALPHA:
> + return nir_channel(b, bconst, 3);
> + case BLEND_FACTOR_SRC_ALPHA_SATURATE: {
> + nir_ssa_def *Asrc = nir_channel(b, src, 3);
> + nir_ssa_def *Adst = nir_channel(b, dst, 3);
> + nir_ssa_def *one = nir_imm_float(b, 1.0);
> + nir_ssa_def *Adsti = nir_fsub(b, one, Adst);
> +
> + return (chan < 3) ? nir_fmin(b, Asrc, Adsti) : one;
> + }
> + default:
> + unreachable("Invalid blend factor");
Same default case comment here as above.
> + }
> +}
> +
> +static nir_ssa_def *
> +nir_blend_factor(
> + nir_builder *b,
> + nir_ssa_def *raw_scalar,
> + nir_ssa_def *src, nir_ssa_def *dst, nir_ssa_def *bconst,
> + unsigned chan,
> + enum blend_factor factor,
> + bool inverted)
> +{
> + nir_ssa_def *f =
> + nir_blend_factor_value(b, src, dst, bconst, chan, factor);
> +
> + if (inverted)
> + f = nir_fsub(b, nir_imm_float(b, 1.0), f);
> +
> + return nir_fmul(b, raw_scalar, f);
> +}
> +
> +/* Given a blend state, the source color, and the destination color,
> + * return the blended color */
> +
> +static nir_ssa_def *
> +nir_blend(
> + nir_builder *b,
> + nir_lower_blend_options options,
> + nir_ssa_def *src, nir_ssa_def *dst)
> +{
> + /* Grab the blend constant ahead of time */
> + nir_ssa_def *bconst = nir_load_blend_const_color_rgba(b);
> +
> + /* We blend per channel and recombine later */
Why? Without a vectorizer (or even with a vectorizer), it seems like
this will generate much worse code. I guess it's only a few
instructions once per shader, so it may not matter... but it's a little
surprising especially after going to extra effort to get the blend color
as a vector.
> + nir_ssa_def *channels[4];
> +
> + for (unsigned c = 0; c < 4; ++c) {
> + /* Decide properties based on channel */
> + nir_lower_blend_channel chan =
> + (c < 3) ? options.rt[0].rgb : options.rt[0].alpha;
> +
> + nir_ssa_def *psrc = nir_channel(b, src, c);
> + nir_ssa_def *pdst = nir_channel(b, dst, c);
> +
> + if (nir_blend_factored(chan.func)) {
> + psrc = nir_blend_factor(
> + b, psrc,
> + src, dst, bconst, c,
> + chan.src_factor, chan.invert_src_factor);
> +
> + pdst = nir_blend_factor(
> + b, pdst,
> + src, dst, bconst, c,
> + chan.dst_factor, chan.invert_dst_factor);
> + }
> +
> + channels[c] = nir_blend_func(b, chan.func, psrc, pdst);
> + }
> +
> + return nir_vec(b, channels, 4);
> +}
> +
> +void
> +nir_lower_blend(nir_shader *shader, nir_lower_blend_options options)
> +{
> + /* Blend shaders are represented as special fragment shaders */
> + assert(shader->info.stage == MESA_SHADER_FRAGMENT);
> +
> + nir_foreach_function(func, shader) {
> + nir_foreach_block(block, func->impl) {
> + nir_foreach_instr_safe(instr, block) {
FWIW, since someone will probably comment, I like this organization
better than the method other passes use of splitting things into
multiple functions.
> + if (instr->type != nir_instr_type_intrinsic)
> + continue;
> +
> + nir_intrinsic_instr *intr = nir_instr_as_intrinsic(instr);
> + if (intr->intrinsic != nir_intrinsic_store_deref)
> + continue;
> +
> + /* TODO: Extending to MRT */
> + nir_variable *var = nir_intrinsic_get_var(intr, 0);
> + if (var->data.location != FRAG_RESULT_COLOR)
> + continue;
> +
> + nir_builder b;
> + nir_builder_init(&b, func->impl);
> + b.cursor = nir_before_instr(instr);
> +
> + /* Grab the input color */
> + nir_ssa_def *src = nir_ssa_for_src(&b, intr->src[1], 4);
> +
> + /* Grab the tilebuffer color - io lowered to load_output */
> + nir_ssa_def *dst = nir_load_var(&b, var);
> +
> + /* Blend and write that instead */
> + nir_ssa_def *blended = nir_blend(&b, options, src, dst);
> +
> + nir_instr_rewrite_src(instr, &intr->src[1],
> + nir_src_for_ssa(blended));
> +
> + }
> + }
> +
> + nir_metadata_preserve(func->impl, nir_metadata_block_index |
> + nir_metadata_dominance);
> + }
> +}
More information about the mesa-dev
mailing list