Mesa (master): intel/nir: Rewrite the guts of lower_alpha_to_coverage
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Sat Aug 29 16:46:10 UTC 2020
Module: Mesa
Branch: master
Commit: b6fdb1405ee2688ffc15acdf0476dece8bc8846b
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=b6fdb1405ee2688ffc15acdf0476dece8bc8846b
Author: Jason Ekstrand <jason at jlekstrand.net>
Date: Fri Aug 7 11:25:54 2020 -0500
intel/nir: Rewrite the guts of lower_alpha_to_coverage
I have no idea how this pass ever worked. I guess it worked ok on the
one or two piglit tests but the whole thing seemed very fragile. It
makes a number of undocumented and unasserted assumptions and they
aren't always valid. This rewrite makes a number of changes:
1. It now properly handles the case where the gl_SampleMask write comes
before the gl_FragColor or gl_FragData[0] write.
2. It should early-exit faster because it now looks at bits in
shader_info::outputs_written instead of looking for variables.
3. Instead of the fragile variable lookup where we try to look the
variable up by both location and driver_location and match, we just
use the driver_location calculations used by brw_fs_nir.
4. It asserts that the index parameter to store_output is a constant
instead of silently failing if it isn't.
5. We now actually assert the implicit assumption that the two writes
are in the same block. We go even further and assert that they are
in the last block in the shader.
6. In the case where 3 or fewer components of the output are written,
we explicitly choose to leave the sample mask alone.
Fixes: 7ecfbd4f6d4 "nir: Add alpha_to_coverage lowering pass"
Closes: #3166
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/6233>
---
src/intel/compiler/brw_nir.h | 2 +-
.../compiler/brw_nir_lower_alpha_to_coverage.c | 164 +++++++++++++--------
2 files changed, 100 insertions(+), 66 deletions(-)
diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h
index 1241e54fc5a..616c4244295 100644
--- a/src/intel/compiler/brw_nir.h
+++ b/src/intel/compiler/brw_nir.h
@@ -100,7 +100,7 @@ brw_nir_link_shaders(const struct brw_compiler *compiler,
nir_shader *producer, nir_shader *consumer);
bool brw_nir_lower_cs_intrinsics(nir_shader *nir);
-void brw_nir_lower_alpha_to_coverage(nir_shader *shader);
+bool brw_nir_lower_alpha_to_coverage(nir_shader *shader);
void brw_nir_lower_legacy_clipping(nir_shader *nir,
int nr_userclip_plane_consts,
struct brw_stage_prog_data *prog_data);
diff --git a/src/intel/compiler/brw_nir_lower_alpha_to_coverage.c b/src/intel/compiler/brw_nir_lower_alpha_to_coverage.c
index c804bdeaab8..d31384e5615 100644
--- a/src/intel/compiler/brw_nir_lower_alpha_to_coverage.c
+++ b/src/intel/compiler/brw_nir_lower_alpha_to_coverage.c
@@ -56,10 +56,10 @@
* 1.0000 1111111111111111
*/
static nir_ssa_def *
-build_dither_mask(nir_builder *b, nir_intrinsic_instr *store_instr)
+build_dither_mask(nir_builder *b, nir_ssa_def *color)
{
- nir_ssa_def *alpha =
- nir_channel(b, nir_ssa_for_src(b, store_instr->src[0], 4), 3);
+ assert(color->num_components == 4);
+ nir_ssa_def *alpha = nir_channel(b, color, 3);
nir_ssa_def *m =
nir_f2i32(b, nir_fmul_imm(b, nir_fsat(b, alpha), 16.0));
@@ -82,74 +82,108 @@ build_dither_mask(nir_builder *b, nir_intrinsic_instr *store_instr)
nir_imul_imm(b, part_c, 0x0100)));
}
-void
+bool
brw_nir_lower_alpha_to_coverage(nir_shader *shader)
{
assert(shader->info.stage == MESA_SHADER_FRAGMENT);
-
- /* Bail out early if we don't have gl_SampleMask */
- if (!nir_find_variable_with_location(shader, nir_var_shader_out,
- FRAG_RESULT_SAMPLE_MASK))
- return;
-
- nir_foreach_function(function, shader) {
- nir_function_impl *impl = function->impl;
- nir_builder b;
- nir_builder_init(&b, impl);
-
- nir_foreach_block(block, impl) {
- nir_intrinsic_instr *sample_mask_instr = NULL;
- nir_intrinsic_instr *store_instr = NULL;
-
- nir_foreach_instr_safe(instr, block) {
- if (instr->type == nir_instr_type_intrinsic) {
- nir_intrinsic_instr *intr = nir_instr_as_intrinsic(instr);
- nir_variable *out = NULL;
-
- switch (intr->intrinsic) {
- case nir_intrinsic_store_output:
- out = nir_find_variable_with_driver_location(shader, nir_var_shader_out,
- nir_intrinsic_base(intr));
- assert(out->data.mode == nir_var_shader_out);
-
- /* save gl_SampleMask instruction pointer */
- if (out->data.location == FRAG_RESULT_SAMPLE_MASK) {
- assert(!sample_mask_instr);
- sample_mask_instr = intr;
- }
-
- /* save out_color[0] instruction pointer */
- if ((out->data.location == FRAG_RESULT_COLOR ||
- out->data.location == FRAG_RESULT_DATA0)) {
- nir_src *offset_src = nir_get_io_offset_src(intr);
- if (nir_src_is_const(*offset_src) && nir_src_as_uint(*offset_src) == 0) {
- assert(!store_instr);
- store_instr = intr;
- }
- }
- break;
- default:
- continue;
- }
- }
+ nir_function_impl *impl = nir_shader_get_entrypoint(shader);
+
+ const uint64_t outputs_written = shader->info.outputs_written;
+ if (!(outputs_written & BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK)) ||
+ !(outputs_written & (BITFIELD64_BIT(FRAG_RESULT_COLOR) |
+ BITFIELD64_BIT(FRAG_RESULT_DATA0))))
+ goto skip;
+
+ nir_intrinsic_instr *sample_mask_write = NULL;
+ nir_intrinsic_instr *color0_write = NULL;
+ bool sample_mask_write_first = false;
+
+ nir_foreach_block(block, impl) {
+ nir_foreach_instr_safe(instr, block) {
+ if (instr->type != nir_instr_type_intrinsic)
+ continue;
+
+ nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
+ if (intrin->intrinsic != nir_intrinsic_store_output)
+ continue;
+
+ /* We call nir_lower_io_to_temporaries to lower FS outputs to
+ * temporaries with a copy at the end so this should be the last
+ * block in the shader.
+ */
+ assert(block->cf_node.parent == &impl->cf_node);
+ assert(nir_cf_node_is_last(&block->cf_node));
+
+ /* See store_output in fs_visitor::nir_emit_fs_intrinsic */
+ const unsigned store_offset = nir_src_as_uint(intrin->src[1]);
+ const unsigned driver_location = nir_intrinsic_base(intrin) +
+ SET_FIELD(store_offset, BRW_NIR_FRAG_OUTPUT_LOCATION);
+
+ /* Extract the FRAG_RESULT */
+ const unsigned location =
+ GET_FIELD(driver_location, BRW_NIR_FRAG_OUTPUT_LOCATION);
+
+ if (location == FRAG_RESULT_SAMPLE_MASK) {
+ assert(sample_mask_write == NULL);
+ sample_mask_write = intrin;
+ sample_mask_write_first = (color0_write == NULL);
}
- if (sample_mask_instr && store_instr) {
- b.cursor = nir_before_instr(&store_instr->instr);
- nir_ssa_def *dither_mask = build_dither_mask(&b, store_instr);
-
- /* Combine dither_mask and reorder gl_SampleMask store instruction
- * after render target 0 store instruction.
- */
- nir_instr_remove(&sample_mask_instr->instr);
- dither_mask = nir_iand(&b, sample_mask_instr->src[0].ssa, dither_mask);
- nir_instr_insert_after(&store_instr->instr, &sample_mask_instr->instr);
- nir_instr_rewrite_src(&sample_mask_instr->instr,
- &sample_mask_instr->src[0],
- nir_src_for_ssa(dither_mask));
+ if (location == FRAG_RESULT_COLOR ||
+ location == FRAG_RESULT_DATA0) {
+ assert(color0_write == NULL);
+ color0_write = intrin;
}
}
- nir_metadata_preserve(impl, nir_metadata_block_index |
- nir_metadata_dominance);
}
+
+ /* It's possible that shader_info may be out-of-date and the writes to
+ * either gl_SampleMask or the first color value may have been removed.
+ * This can happen if, for instance a nir_ssa_undef is written to the
+ * color value. In that case, just bail and don't do anything rather
+ * than crashing.
+ */
+ if (color0_write == NULL || sample_mask_write == NULL)
+ goto skip;
+
+ /* It's possible that the color value isn't actually a vec4. In this case,
+ * assuming an alpha of 1.0 and letting the sample mask pass through
+ * unaltered seems like the kindest thing to do to apps.
+ */
+ assert(color0_write->src[0].is_ssa);
+ nir_ssa_def *color0 = color0_write->src[0].ssa;
+ if (color0->num_components < 4)
+ goto skip;
+
+ assert(sample_mask_write->src[0].is_ssa);
+ nir_ssa_def *sample_mask = sample_mask_write->src[0].ssa;
+
+ if (sample_mask_write_first) {
+ /* If the sample mask write comes before the write to color0, we need
+ * to move it because it's going to use the value from color0 to
+ * compute the sample mask.
+ */
+ nir_instr_remove(&sample_mask_write->instr);
+ nir_instr_insert(nir_after_instr(&color0_write->instr),
+ &sample_mask_write->instr);
+ }
+
+ nir_builder b;
+ nir_builder_init(&b, impl);
+
+ /* Combine dither_mask and the gl_SampleMask value */
+ b.cursor = nir_before_instr(&sample_mask_write->instr);
+ nir_ssa_def *dither_mask = build_dither_mask(&b, color0);
+ dither_mask = nir_iand(&b, sample_mask, dither_mask);
+ nir_instr_rewrite_src(&sample_mask_write->instr,
+ &sample_mask_write->src[0],
+ nir_src_for_ssa(dither_mask));
+
+ nir_metadata_preserve(impl, nir_metadata_block_index |
+ nir_metadata_dominance);
+ return true;
+
+skip:
+ nir_metadata_preserve(impl, nir_metadata_all);
+ return false;
}
More information about the mesa-commit
mailing list