From 13ceea551cf833ad29fe7bef7c0c2811efaec357 Mon Sep 17 00:00:00 2001 From: Sviatoslav Peleshko Date: Mon, 24 Apr 2023 07:57:24 +0300 Subject: [PATCH] nir: Use alu source components count in nir_alu_srcs_negative_equal When we use source from ALU instruction directly, the default swizzle array should be populated with the same amount of components as the src has. Otherwise, if we use nir_ssa_alu_instr_src_components, it can return the destination components count that is lower than component index actually used in that source. This can lead to false equality between 0 (uninitialized) and 0 (.x) in swizzle comparison below. Fixes: c6ee46a7 ("nir: Add nir_alu_srcs_negative_equal") Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/8704 Signed-off-by: Sviatoslav Peleshko Reviewed-by: Ian Romanick Part-of: (cherry picked from commit 6b0bfdfa9e83aba5316821296fb9cf68fdc958f8) --- .pick_status.json | 2 +- src/compiler/nir/nir_instr_set.c | 4 +- .../nir/tests/comparison_pre_tests.cpp | 92 +++++++++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index bac2edd2de3..54022b3c3d9 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -484,7 +484,7 @@ "description": "nir: Use alu source components count in nir_alu_srcs_negative_equal", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "c6ee46a7532291fc8583400e174e77b1833daf23", "notes": null diff --git a/src/compiler/nir/nir_instr_set.c b/src/compiler/nir/nir_instr_set.c index ad0558d8be4..caef18f1e1d 100644 --- a/src/compiler/nir/nir_instr_set.c +++ b/src/compiler/nir/nir_instr_set.c @@ -441,7 +441,7 @@ nir_alu_srcs_negative_equal(const nir_alu_instr *alu1, } else { alu1_actual_src = alu1->src[src1].src; - for (unsigned i = 0; i < nir_ssa_alu_instr_src_components(alu1, src1); i++) + for (unsigned i = 0; i < nir_src_num_components(alu1_actual_src); i++) alu1_swizzle[i] = i; } @@ -458,7 +458,7 @@ nir_alu_srcs_negative_equal(const nir_alu_instr *alu1, } else { alu2_actual_src = alu2->src[src2].src; - for (unsigned i = 0; i < nir_ssa_alu_instr_src_components(alu2, src2); i++) + for (unsigned i = 0; i < nir_src_num_components(alu2_actual_src); i++) alu2_swizzle[i] = i; } diff --git a/src/compiler/nir/tests/comparison_pre_tests.cpp b/src/compiler/nir/tests/comparison_pre_tests.cpp index 78e08d27a6d..e969719a55c 100644 --- a/src/compiler/nir/tests/comparison_pre_tests.cpp +++ b/src/compiler/nir/tests/comparison_pre_tests.cpp @@ -579,3 +579,95 @@ TEST_F(comparison_pre_test, non_scalar_add_result) EXPECT_FALSE(nir_opt_comparison_pre_impl(bld.impl)); } + +TEST_F(comparison_pre_test, multi_comps_load) +{ + /* Before: + * + * vec1 32 ssa_0 = load_ubo (...) + * vec4 32 ssa_1 = load_ubo (...) + * vec1 1 ssa_2 = flt ssa_0, ssa_1.w + * + * if ssa_2 { + * vec1 32 ssa_3 = fneg ssa_1.x + * vec1 32 ssa_4 = fadd ssa_0, ssa_3 + * } else { + * } + */ + nir_def *ssa_0 = nir_load_ubo(&bld, 1, 32, + nir_imm_int(&bld, 0), + nir_imm_int(&bld, 0)); + nir_def *ssa_1 = nir_load_ubo(&bld, 4, 32, + nir_imm_int(&bld, 1), + nir_imm_int(&bld, 0)); + + nir_alu_instr *flt = nir_alu_instr_create(bld.shader, nir_op_flt); + flt->src[0].src = nir_src_for_ssa(ssa_0); + flt->src[1].src = nir_src_for_ssa(ssa_1); + memcpy(&flt->src[0].swizzle, xxxx, sizeof(xxxx)); + memcpy(&flt->src[1].swizzle, wwww, sizeof(wwww)); + nir_builder_alu_instr_finish_and_insert(&bld, flt); + flt->def.num_components = 1; + nir_def *ssa_2 = &flt->def; + + nir_if *nif = nir_push_if(&bld, ssa_2); + { + nir_alu_instr *fneg = nir_alu_instr_create(bld.shader, nir_op_fneg); + fneg->src[0].src = nir_src_for_ssa(ssa_1); + memcpy(&fneg->src[0].swizzle, xxxx, sizeof(xxxx)); + nir_builder_alu_instr_finish_and_insert(&bld, fneg); + fneg->def.num_components = 1; + nir_def *ssa_3 = &fneg->def; + + nir_fadd(&bld, ssa_0, ssa_3); + } + nir_pop_if(&bld, nif); + + EXPECT_FALSE(nir_opt_comparison_pre_impl(bld.impl)); +} + +TEST_F(comparison_pre_test, multi_comps_load2) +{ + /* Before: + * + * vec1 32 ssa_0 = load_ubo (...) + * vec4 32 ssa_1 = load_ubo (...) + * vec1 1 ssa_2 = flt ssa_0, ssa_1.x + * + * if ssa_2 { + * vec1 32 ssa_3 = fneg ssa_1.w + * vec1 32 ssa_4 = fadd ssa_0, ssa_3 + * } else { + * } + */ + nir_def *ssa_0 = nir_load_ubo(&bld, 1, 32, + nir_imm_int(&bld, 0), + nir_imm_int(&bld, 0)); + nir_def *ssa_1 = nir_load_ubo(&bld, 4, 32, + nir_imm_int(&bld, 1), + nir_imm_int(&bld, 0)); + + nir_alu_instr *flt = nir_alu_instr_create(bld.shader, nir_op_flt); + flt->src[0].src = nir_src_for_ssa(ssa_0); + flt->src[1].src = nir_src_for_ssa(ssa_1); + memcpy(&flt->src[0].swizzle, xxxx, sizeof(xxxx)); + memcpy(&flt->src[1].swizzle, xxxx, sizeof(xxxx)); + nir_builder_alu_instr_finish_and_insert(&bld, flt); + flt->def.num_components = 1; + nir_def *ssa_2 = &flt->def; + + nir_if *nif = nir_push_if(&bld, ssa_2); + { + nir_alu_instr *fneg = nir_alu_instr_create(bld.shader, nir_op_fneg); + fneg->src[0].src = nir_src_for_ssa(ssa_1); + memcpy(&fneg->src[0].swizzle, wwww, sizeof(wwww)); + nir_builder_alu_instr_finish_and_insert(&bld, fneg); + fneg->def.num_components = 1; + nir_def *ssa_3 = &fneg->def; + + nir_fadd(&bld, ssa_0, ssa_3); + } + nir_pop_if(&bld, nif); + + EXPECT_FALSE(nir_opt_comparison_pre_impl(bld.impl)); +}