Module: Mesa Branch: main Commit: 5adb33550737fe2add6cedbe2d43613de9530a5f URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=5adb33550737fe2add6cedbe2d43613de9530a5f
Author: Faith Ekstrand <[email protected]> Date: Wed Apr 19 11:34:39 2023 -0500 nir: Use nir_builder to insert movs Also, leave a big comment about why we're inserting movs and not just propagating SSA values directly. Hopefully this will prevent idiots like me from getting clever and thinking they can delete that mov. 😅 Reviewed-by: Karol Herbst <[email protected]> Reviewed-by: Alyssa Rosenzweig <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22580> --- src/compiler/nir/nir_lower_vars_to_ssa.c | 36 +++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/compiler/nir/nir_lower_vars_to_ssa.c b/src/compiler/nir/nir_lower_vars_to_ssa.c index 468aef529cb..b9adc17c8ae 100644 --- a/src/compiler/nir/nir_lower_vars_to_ssa.c +++ b/src/compiler/nir/nir_lower_vars_to_ssa.c @@ -598,21 +598,33 @@ rename_variables(struct lower_variables_state *state) if (!node->lower_to_ssa) continue; - nir_alu_instr *mov = nir_alu_instr_create(state->shader, - nir_op_mov); - mov->src[0].src = nir_src_for_ssa( - nir_phi_builder_value_get_block_def(node->pb_value, block)); - for (unsigned i = intrin->num_components; i < NIR_MAX_VEC_COMPONENTS; i++) - mov->src[0].swizzle[i] = 0; + nir_def *val = + nir_phi_builder_value_get_block_def(node->pb_value, block); + + /* As tempting as it is to just rewrite the uses of our load + * instruction with the value we got out of the phi builder, we + * can't do that without risking messing ourselves up. In + * particular, the get_deref_node() function we call during + * variable renaming uses nir_src_is_const() to determine which + * deref node to fetch. If we propagate directly, we may end up + * propagating a constant into an array index, changing the + * behavior of get_deref_node() for that deref and invalidating + * our analysis. + * + * With enough work, we could probably make our analysis and data + * structures robust against this but it would make everything + * more complicated to reason about. It's easier to just insert + * a mov and let copy-prop clean up after us. This pass is + * complicated enough as-is. + */ + b.cursor = nir_before_instr(&intrin->instr); + val = nir_mov(&b, val); - nir_def_init(&mov->instr, &mov->def, - intrin->num_components, intrin->def.bit_size); + assert(val->bit_size == intrin->def.bit_size); + assert(val->num_components == intrin->def.num_components); - nir_instr_insert_before(&intrin->instr, &mov->instr); + nir_def_rewrite_uses(&intrin->def, val); nir_instr_remove(&intrin->instr); - - nir_def_rewrite_uses(&intrin->def, - &mov->def); break; }
