Mesa (main): pan/bi: Validate the live set starts empty
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Thu Jul 29 23:53:26 UTC 2021
Module: Mesa
Branch: main
Commit: 1e29f57b3ac4ec6560d0d355576be3cd784cf131
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=1e29f57b3ac4ec6560d0d355576be3cd784cf131
Author: Alyssa Rosenzweig <alyssa at collabora.com>
Date: Tue Jul 27 18:34:35 2021 -0400
pan/bi: Validate the live set starts empty
Otherwise there is an uninitialized read, and the register allocation
will fail. (In the sense of failing a precondition. This manifests as
synthetic interference leading to higher register pressure and useless
moves. The allocation itself is ok, but it indicates a real bug.)
Signed-off-by: Alyssa Rosenzweig <alyssa at collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12130>
---
src/panfrost/bifrost/bi_validate.c | 78 ++++++++++++++++++++++++++++++++++
src/panfrost/bifrost/bifrost.h | 1 +
src/panfrost/bifrost/bifrost_compile.c | 6 +++
src/panfrost/bifrost/compiler.h | 8 ++++
src/panfrost/bifrost/meson.build | 1 +
5 files changed, 94 insertions(+)
diff --git a/src/panfrost/bifrost/bi_validate.c b/src/panfrost/bifrost/bi_validate.c
new file mode 100644
index 00000000000..d6eab037543
--- /dev/null
+++ b/src/panfrost/bifrost/bi_validate.c
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2021 Collabora, Ltd.
+ *
+ * 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.
+ */
+
+#include "compiler.h"
+#include "util/u_memory.h"
+
+/* Validatation doesn't make sense in release builds */
+#ifndef NDEBUG
+
+/* Validate that all sources are initialized in all read components. This is
+ * required for correct register allocation. We check a weaker condition, that
+ * all sources that are read are written at some point (equivalently, the live
+ * set is empty at the start of the program). TODO: Strengthen */
+
+bool
+bi_validate_initialization(bi_context *ctx)
+{
+ bool success = true;
+
+ /* Calculate the live set */
+ bi_block *entry = bi_entry_block(ctx);
+ unsigned temp_count = bi_max_temp(ctx);
+ bi_invalidate_liveness(ctx);
+ bi_compute_liveness(ctx);
+
+ /* Validate that the live set is indeed empty */
+ for (unsigned i = 0; i < temp_count; ++i) {
+ if (entry->live_in[i] == 0) continue;
+
+ fprintf(stderr, "%s%u\n", (i & PAN_IS_REG) ? "r" : "", i >> 1);
+ success = false;
+ }
+
+ return success;
+}
+
+void
+bi_validate(bi_context *ctx, const char *after)
+{
+ bool fail = false;
+
+ if (bifrost_debug & BIFROST_DBG_NOVALIDATE)
+ return;
+
+ if (!bi_validate_initialization(ctx)) {
+ fprintf(stderr, "Uninitialized data read after %s\n", after);
+ fail = true;
+ }
+
+ /* TODO: Validate more invariants */
+
+ if (fail) {
+ bi_print_shader(ctx, stderr);
+ exit(1);
+ }
+}
+
+#endif /* NDEBUG */
diff --git a/src/panfrost/bifrost/bifrost.h b/src/panfrost/bifrost/bifrost.h
index 436404f0be0..74b110f204c 100644
--- a/src/panfrost/bifrost/bifrost.h
+++ b/src/panfrost/bifrost/bifrost.h
@@ -37,6 +37,7 @@
#define BIFROST_DBG_INTERNAL 0x0010
#define BIFROST_DBG_NOSCHED 0x0020
#define BIFROST_DBG_INORDER 0x0040
+#define BIFROST_DBG_NOVALIDATE 0x0080
extern int bifrost_debug;
diff --git a/src/panfrost/bifrost/bifrost_compile.c b/src/panfrost/bifrost/bifrost_compile.c
index cc6bb8741fb..afacef28831 100644
--- a/src/panfrost/bifrost/bifrost_compile.c
+++ b/src/panfrost/bifrost/bifrost_compile.c
@@ -45,6 +45,7 @@ static const struct debug_named_value bifrost_debug_options[] = {
{"internal", BIFROST_DBG_INTERNAL, "Dump even internal shaders"},
{"nosched", BIFROST_DBG_NOSCHED, "Force trivial bundling"},
{"inorder", BIFROST_DBG_INORDER, "Force in-order bundling"},
+ {"novalidate",BIFROST_DBG_NOVALIDATE, "Skip IR validation"},
DEBUG_NAMED_VALUE_END
};
@@ -3659,6 +3660,8 @@ bifrost_compile_shader_nir(nir_shader *nir,
block->name = block_source_count++;
}
+ bi_validate(ctx, "NIR -> BIR");
+
/* If the shader doesn't write any colour or depth outputs, it may
* still need an ATEST at the very end! */
bool need_dummy_atest =
@@ -3674,6 +3677,7 @@ bifrost_compile_shader_nir(nir_shader *nir,
/* Runs before constant folding */
bi_lower_swizzle(ctx);
+ bi_validate(ctx, "Early lowering");
/* Runs before copy prop */
bi_opt_push_ubo(ctx);
@@ -3685,6 +3689,7 @@ bifrost_compile_shader_nir(nir_shader *nir,
bi_opt_dead_code_eliminate(ctx);
bi_opt_cse(ctx);
bi_opt_dead_code_eliminate(ctx);
+ bi_validate(ctx, "Optimization passes");
bi_foreach_block(ctx, block) {
bi_lower_branch(block);
@@ -3698,6 +3703,7 @@ bifrost_compile_shader_nir(nir_shader *nir,
* skip bit is a function of only the data flow graph and is invariant
* under valid scheduling. */
bi_analyze_helper_requirements(ctx);
+ bi_validate(ctx, "Late lowering");
bi_register_allocate(ctx);
bi_opt_post_ra(ctx);
diff --git a/src/panfrost/bifrost/compiler.h b/src/panfrost/bifrost/compiler.h
index b1d50cc7945..311c99a2730 100644
--- a/src/panfrost/bifrost/compiler.h
+++ b/src/panfrost/bifrost/compiler.h
@@ -910,6 +910,14 @@ bool bi_reads_zero(bi_instr *ins);
bool bi_reads_temps(bi_instr *ins, unsigned src);
bool bi_reads_t(bi_instr *ins, unsigned src);
+#ifndef NDEBUG
+bool bi_validate_initialization(bi_context *ctx);
+void bi_validate(bi_context *ctx, const char *after_str);
+#else
+static inline bool bi_validate_initialization(UNUSED bi_context *ctx) { return true; }
+static inline void bi_validate(UNUSED bi_context *ctx, UNUSED const char *after_str) { return; }
+#endif
+
uint32_t bi_fold_constant(bi_instr *I, bool *unsupported);
void bi_opt_constant_fold(bi_context *ctx);
diff --git a/src/panfrost/bifrost/meson.build b/src/panfrost/bifrost/meson.build
index e18d5a57239..38f15391f45 100644
--- a/src/panfrost/bifrost/meson.build
+++ b/src/panfrost/bifrost/meson.build
@@ -38,6 +38,7 @@ libpanfrost_bifrost_files = files(
'bi_ra.c',
'bi_schedule.c',
'bi_scoreboard.c',
+ 'bi_validate.c',
'bir.c',
'bifrost_compile.c',
)
More information about the mesa-commit
mailing list