nir/lcssa: fix premature exit of loop after rematerializing derefs

If we have NIR such as:

32x4  %48 = @load_vulkan_descriptor (%47) (desc_type=SSBO)
32x4  %76 = deref_cast (tint_symbol_11 *)%48 (ssbo tint_symbol_11)  (ptr_stride=0, align_mul=4, align_offset=0)
32x4  %77 = deref_struct &%76->tint_symbol_10 (ssbo int)  // &((tint_symbol_11 *)%48)->tint_symbol_10

A single nir_rematerialize_deref_in_use_blocks() will rematerialize the
deref_struct and then it's deref_cast. However,
nir_foreach_instr_reverse_safe is not safe if the next iteration's
instruction is removed. This can result in the instruction loop exiting
and the load_vulkan_descriptor never having an LCSSA phi.

Signed-off-by: Rhys Perry <pendingchaos02@gmail.com>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Fixes: 439e8c42cc ("nir/lcssa: Fix rematerializing derefs")
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/11770
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/32225>
This commit is contained in:
Rhys Perry 2024-10-17 10:21:47 +01:00 committed by Marge Bot
parent 327e5465fc
commit 65a54b4ec4
2 changed files with 61 additions and 13 deletions

View File

@ -280,6 +280,28 @@ setup_loop_state(lcssa_state *state, nir_loop *loop)
state->exit_blocks = nir_block_get_predecessors_sorted(state->block_after_loop, state);
}
static void
convert_block_to_lcssa(nir_block *block, lcssa_state *state)
{
nir_instr *instr = nir_block_last_instr(block);
while (instr) {
/* We assume "next" will not be removed. It is used to obtain the next iteration's
* pointer if "instr" is removed.
*/
nir_instr *next = nir_instr_next(instr);
nir_foreach_def(instr, convert_loop_exit_for_ssa, state);
/* for outer loops, invariant instructions can be variant */
if (state->skip_invariants && instr->pass_flags == invariant)
instr->pass_flags = undefined;
nir_instr *if_removed = next ? nir_instr_prev(next) : nir_block_last_instr(block);
bool has_instr_been_removed = if_removed != instr;
instr = has_instr_been_removed ? if_removed : nir_instr_prev(instr);
}
}
static void
convert_to_lcssa(nir_cf_node *cf_node, lcssa_state *state)
{
@ -328,15 +350,8 @@ convert_to_lcssa(nir_cf_node *cf_node, lcssa_state *state)
}
}
nir_foreach_block_in_cf_node_reverse(block, cf_node) {
nir_foreach_instr_reverse_safe(instr, block) {
nir_foreach_def(instr, convert_loop_exit_for_ssa, state);
/* for outer loops, invariant instructions can be variant */
if (state->skip_invariants && instr->pass_flags == invariant)
instr->pass_flags = undefined;
}
}
nir_foreach_block_in_cf_node_reverse(block, cf_node)
convert_block_to_lcssa(block, state);
end:
/* For outer loops, the LCSSA-phi should be considered not invariant */
@ -369,10 +384,8 @@ nir_convert_loop_to_lcssa(nir_loop *loop)
state->skip_invariants = false;
state->skip_bool_invariants = false;
nir_foreach_block_in_cf_node_reverse(block, &loop->cf_node) {
nir_foreach_instr_reverse_safe(instr, block)
nir_foreach_def(instr, convert_loop_exit_for_ssa, state);
}
nir_foreach_block_in_cf_node_reverse(block, &loop->cf_node)
convert_block_to_lcssa(block, state);
ralloc_free(state);
}

View File

@ -130,3 +130,38 @@ TEST_F(nir_cf_test, delete_break_in_loop)
nir_metadata_require(b->impl, nir_metadata_dominance);
}
/* This tests a bug in nir_convert_loop_to_lcssa, two consecutive deref
* instructions can cause the function's loop to exit prematurely if
* rematerializing the second causes the first to also be rematerialized. */
TEST_F(nir_cf_test, lcssa_iter_safety_during_deref_remat)
{
nir_variable *ubo_var_array = nir_variable_create(
b->shader, nir_var_mem_ubo, glsl_array_type(glsl_int_type(), 4, 0), "ubo_array");
nir_variable *out_var = nir_variable_create(
b->shader, nir_var_shader_out, glsl_int_type(), "out");
nir_loop *loop = nir_push_loop(b);
nir_def *index = nir_imm_int(b, 2);
nir_deref_instr *deref = nir_build_deref_var(b, ubo_var_array);
deref = nir_build_deref_array(b, deref, index);
nir_jump(b, nir_jump_break);
nir_pop_loop(b, loop);
nir_def *val = nir_load_deref(b, deref);
nir_store_deref(b, nir_build_deref_var(b, out_var), val, 0x1);
nir_convert_loop_to_lcssa(loop);
nir_block *block_after_loop = nir_cf_node_as_block(nir_cf_node_next(&loop->cf_node));
EXPECT_FALSE(nir_def_is_unused(index));
nir_foreach_use_including_if(src, index)
EXPECT_TRUE(!nir_src_is_if(src) && nir_src_parent_instr(src)->type == nir_instr_type_phi &&
nir_src_parent_instr(src)->block == block_after_loop);
nir_validate_shader(b->shader, NULL);
nir_validate_ssa_dominance(b->shader, NULL);
}