[Mesa-dev] [PATCH 2/5] i965/vec4/dce: Don't narrow the write mask if the flags are used
Ian Romanick
idr at freedesktop.org
Wed Aug 29 18:40:27 UTC 2018
From: Ian Romanick <ian.d.romanick at intel.com>
In an instruction sequence like
cmp(8).ge.f0.0 vgrf17:D, vgrf2.xxxx:D, vgrf9.xxxx:D
(+f0.0) sel(8) vgrf1:UD, vgrf8.xyzw:UD, vgrf1.xyzw:UD
The other fields of vgrf17 may be unused, but the CMP still needs to
generate the other flag bits.
To my surprise, nothing in shader-db or any test suite appears to hit
this. However, I have a change to brw_vec4_cmod_propagation that
creates cases where this can happen. This fix prevents a couple dozen
regressions in that patch.
Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
---
src/intel/Makefile.compiler.am | 5 +
.../compiler/brw_vec4_dead_code_eliminate.cpp | 47 ++++--
src/intel/compiler/meson.build | 3 +-
.../compiler/test_vec4_dead_code_eliminate.cpp | 163 +++++++++++++++++++++
4 files changed, 208 insertions(+), 10 deletions(-)
create mode 100644 src/intel/compiler/test_vec4_dead_code_eliminate.cpp
diff --git a/src/intel/Makefile.compiler.am b/src/intel/Makefile.compiler.am
index 46711fe71b7..2d66883f35a 100644
--- a/src/intel/Makefile.compiler.am
+++ b/src/intel/Makefile.compiler.am
@@ -64,6 +64,7 @@ COMPILER_TESTS = \
compiler/test_vf_float_conversions \
compiler/test_vec4_cmod_propagation \
compiler/test_vec4_copy_propagation \
+ compiler/test_vec4_dead_code_eliminate \
compiler/test_vec4_register_coalesce
TESTS += $(COMPILER_TESTS)
@@ -97,6 +98,10 @@ compiler_test_vec4_cmod_propagation_SOURCES = \
compiler/test_vec4_cmod_propagation.cpp
compiler_test_vec4_cmod_propagation_LDADD = $(TEST_LIBS)
+compiler_test_vec4_dead_code_eliminate_SOURCES = \
+ compiler/test_vec4_dead_code_eliminate.cpp
+compiler_test_vec4_dead_code_eliminate_LDADD = $(TEST_LIBS)
+
# Strictly speaking this is neither a C++ test nor using gtest - we can address
# address that at a later point. Until then, this allows us a to simplify things.
compiler_test_eu_compact_SOURCES = \
diff --git a/src/intel/compiler/brw_vec4_dead_code_eliminate.cpp b/src/intel/compiler/brw_vec4_dead_code_eliminate.cpp
index c09a3d7ebe9..99e4c9cacaf 100644
--- a/src/intel/compiler/brw_vec4_dead_code_eliminate.cpp
+++ b/src/intel/compiler/brw_vec4_dead_code_eliminate.cpp
@@ -81,17 +81,46 @@ vec4_visitor::dead_code_eliminate()
result_live[3] = result;
}
- for (int c = 0; c < 4; c++) {
- if (!result_live[c] && inst->dst.writemask & (1 << c)) {
- inst->dst.writemask &= ~(1 << c);
+ if (inst->writes_flag()) {
+ /* Independently calculate the usage of the flag components and
+ * the destination value components.
+ */
+ uint8_t flag_mask = inst->dst.writemask;
+ uint8_t dest_mask = inst->dst.writemask;
+
+ for (int c = 0; c < 4; c++) {
+ if (!result_live[c] && dest_mask & (1 << c))
+ dest_mask &= ~(1 << c);
+
+ if (!BITSET_TEST(flag_live, c))
+ flag_mask &= ~(1 << c);
+ }
+
+ if (inst->dst.writemask != (flag_mask | dest_mask)) {
progress = true;
+ inst->dst.writemask = flag_mask | dest_mask;
+ }
- if (inst->dst.writemask == 0) {
- if (inst->writes_accumulator || inst->writes_flag()) {
- inst->dst = dst_reg(retype(brw_null_reg(), inst->dst.type));
- } else {
- inst->opcode = BRW_OPCODE_NOP;
- break;
+ /* If none of the destination components are read, replace the
+ * destination register with the NULL register.
+ */
+ if (dest_mask == 0) {
+ progress = true;
+ inst->dst = dst_reg(retype(brw_null_reg(), inst->dst.type));
+ }
+ } else {
+ for (int c = 0; c < 4; c++) {
+ if (!result_live[c] && inst->dst.writemask & (1 << c)) {
+ inst->dst.writemask &= ~(1 << c);
+ progress = true;
+
+ if (inst->dst.writemask == 0) {
+ if (inst->writes_accumulator) {
+ inst->dst = dst_reg(retype(brw_null_reg(), inst->dst.type));
+ } else {
+ inst->opcode = BRW_OPCODE_NOP;
+ break;
+ }
}
}
}
diff --git a/src/intel/compiler/meson.build b/src/intel/compiler/meson.build
index 98860c94374..484f5a076b5 100644
--- a/src/intel/compiler/meson.build
+++ b/src/intel/compiler/meson.build
@@ -144,7 +144,8 @@ if with_tests
foreach t : ['fs_cmod_propagation', 'fs_copy_propagation',
'fs_saturate_propagation', 'vf_float_conversions',
'vec4_register_coalesce', 'vec4_copy_propagation',
- 'vec4_cmod_propagation', 'eu_compact', 'eu_validate']
+ 'vec4_cmod_propagation', 'vec4_dead_code_eliminate',
+ 'eu_compact', 'eu_validate']
test(
t,
executable(
diff --git a/src/intel/compiler/test_vec4_dead_code_eliminate.cpp b/src/intel/compiler/test_vec4_dead_code_eliminate.cpp
new file mode 100644
index 00000000000..25739c2895a
--- /dev/null
+++ b/src/intel/compiler/test_vec4_dead_code_eliminate.cpp
@@ -0,0 +1,163 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * 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 <gtest/gtest.h>
+#include "brw_vec4.h"
+#include "program/program.h"
+
+using namespace brw;
+
+class dead_code_eliminate_test : public ::testing::Test {
+ virtual void SetUp();
+
+public:
+ struct brw_compiler *compiler;
+ struct gen_device_info *devinfo;
+ struct gl_context *ctx;
+ struct gl_shader_program *shader_prog;
+ struct brw_vue_prog_data *prog_data;
+ vec4_visitor *v;
+};
+
+class dead_code_eliminate_vec4_visitor : public vec4_visitor
+{
+public:
+ dead_code_eliminate_vec4_visitor(struct brw_compiler *compiler,
+ nir_shader *shader,
+ struct brw_vue_prog_data *prog_data)
+ : vec4_visitor(compiler, NULL, NULL, prog_data, shader, NULL,
+ false /* no_spills */, -1)
+ {
+ prog_data->dispatch_mode = DISPATCH_MODE_4X2_DUAL_OBJECT;
+ }
+
+protected:
+ virtual dst_reg *make_reg_for_system_value(int /* location */)
+ {
+ unreachable("Not reached");
+ }
+
+ virtual void setup_payload()
+ {
+ unreachable("Not reached");
+ }
+
+ virtual void emit_prolog()
+ {
+ unreachable("Not reached");
+ }
+
+ virtual void emit_thread_end()
+ {
+ unreachable("Not reached");
+ }
+
+ virtual void emit_urb_write_header(int /* mrf */)
+ {
+ unreachable("Not reached");
+ }
+
+ virtual vec4_instruction *emit_urb_write_opcode(bool /* complete */)
+ {
+ unreachable("Not reached");
+ }
+};
+
+
+void dead_code_eliminate_test::SetUp()
+{
+ ctx = (struct gl_context *)calloc(1, sizeof(*ctx));
+ compiler = (struct brw_compiler *)calloc(1, sizeof(*compiler));
+ devinfo = (struct gen_device_info *)calloc(1, sizeof(*devinfo));
+ prog_data = (struct brw_vue_prog_data *)calloc(1, sizeof(*prog_data));
+ compiler->devinfo = devinfo;
+
+ nir_shader *shader =
+ nir_shader_create(NULL, MESA_SHADER_VERTEX, NULL, NULL);
+
+ v = new dead_code_eliminate_vec4_visitor(compiler, shader, prog_data);
+
+ devinfo->gen = 4;
+}
+
+static void
+dead_code_eliminate(vec4_visitor *v)
+{
+ bool print = false;
+
+ if (print) {
+ fprintf(stderr, "instructions before:\n");
+ v->dump_instructions();
+ }
+
+ v->calculate_cfg();
+ v->dead_code_eliminate();
+
+ if (print) {
+ fprintf(stderr, "instructions after:\n");
+ v->dump_instructions();
+ }
+}
+
+TEST_F(dead_code_eliminate_test, some_dead_channels_all_flags_used)
+{
+ const vec4_builder bld = vec4_builder(v).at_end();
+ src_reg r1 = src_reg(v, glsl_type::vec4_type);
+ src_reg r2 = src_reg(v, glsl_type::vec4_type);
+ src_reg r3 = src_reg(v, glsl_type::vec4_type);
+ src_reg r4 = src_reg(v, glsl_type::vec4_type);
+ src_reg r5 = src_reg(v, glsl_type::vec4_type);
+ src_reg r6 = src_reg(v, glsl_type::vec4_type);
+
+ /* Sequence like the following should not be modified by DCE.
+ *
+ * cmp.l.f0(8) g4<1>F g2<4,4,1>.wF g1<4,4,1>.xF
+ * mov(8) g5<1>.xF g4<4,4,1>.xF
+ * (+f0.x) sel(8) g6<1>UD g3<4>UD g6<4>UD
+ */
+ vec4_instruction *test_cmp =
+ bld.CMP(dst_reg(r4), r2, r1, BRW_CONDITIONAL_L);
+
+ test_cmp->src[0].swizzle = BRW_SWIZZLE_WWWW;
+ test_cmp->src[1].swizzle = BRW_SWIZZLE_XXXX;
+
+ vec4_instruction *test_mov =
+ bld.MOV(dst_reg(r5), r4);
+
+ test_mov->dst.writemask = WRITEMASK_X;
+ test_mov->src[0].swizzle = BRW_SWIZZLE_XXXX;
+
+ vec4_instruction *test_sel =
+ bld.SEL(dst_reg(r6), r3, r6);
+
+ set_predicate(BRW_PREDICATE_NORMAL, test_sel);
+
+ /* The scratch write is here just to make r5 and r6 be live so that the
+ * whole program doesn't get eliminated by DCE.
+ */
+ v->emit(v->SCRATCH_WRITE(dst_reg(r4), r6, r5));
+
+ dead_code_eliminate(v);
+
+ EXPECT_EQ(test_cmp->dst.writemask, WRITEMASK_XYZW);
+}
--
2.14.4
More information about the mesa-dev
mailing list