From d3e321c85317646efd14a01f14dba9fd2d92cd20 Mon Sep 17 00:00:00 2001 From: Boris Brezillon Date: Thu, 28 Apr 2022 10:27:17 -0700 Subject: [PATCH] microsoft/compiler: Make input_mappings immune to driver_location holes Right now, nir_to_dxil() assumes driver_location on inputs will be contiguous, which is true for GL, and also true for Vulkan shaders with the current implementation. But we are trying to delegate the varying linking step to Dozen, and that means the driver will assign the driver_location field. For everything except vertex shaders this works fine, because we are in control of the ID we assign to each variable, and can make sure no holes exists in this assignment, but vertex inputs expect the index value (which is directly extracted from the driver_location field) to match the input index defined at pipeline creation time. The compiler has a hack to treat Vulkan differently and extract the index from the var->data.location field instead, but that's a bit confusing. Moreover, the input_mappings[] array is already indexed with the var->data.driver_location field in the input load emission path, so it makes sense to index it with the same field when emitting signatures. Reviewed-by: Jesse Natalie Part-of: --- src/microsoft/compiler/dxil_module.h | 11 ++++++++++- src/microsoft/compiler/dxil_signature.c | 6 +----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/microsoft/compiler/dxil_module.h b/src/microsoft/compiler/dxil_module.h index 701336ec4bc..13323280d03 100644 --- a/src/microsoft/compiler/dxil_module.h +++ b/src/microsoft/compiler/dxil_module.h @@ -197,7 +197,16 @@ struct dxil_module { struct dxil_signature_record inputs[DXIL_SHADER_MAX_IO_ROWS]; struct dxil_signature_record outputs[DXIL_SHADER_MAX_IO_ROWS]; struct dxil_signature_record patch_consts[DXIL_SHADER_MAX_IO_ROWS]; - unsigned input_mappings[DXIL_SHADER_MAX_IO_ROWS]; + + /* This array is indexed using var->data.driver_location, which + * is not a direct match to IO rows, since a row is a vec4, and + * variables can occupy less than that, and several vars can + * be packed in a row. Hence the x4, but I doubt we can end up + * with more than 80x4 variables in practice. Maybe this array + * should be allocated dynamically based on on the maximum + * driver_location across all input vars. + */ + unsigned input_mappings[DXIL_SHADER_MAX_IO_ROWS * 4]; struct dxil_psv_signature_element psv_inputs[DXIL_SHADER_MAX_IO_ROWS]; struct dxil_psv_signature_element psv_outputs[DXIL_SHADER_MAX_IO_ROWS]; diff --git a/src/microsoft/compiler/dxil_signature.c b/src/microsoft/compiler/dxil_signature.c index 732ad42bf4e..b96e92c14fd 100644 --- a/src/microsoft/compiler/dxil_signature.c +++ b/src/microsoft/compiler/dxil_signature.c @@ -550,11 +550,7 @@ get_input_signature_group(struct dxil_module *mod, const struct dxil_mdnode **in mod->inputs[num_inputs].sysvalue = semantic.sysvalue_name; *row_iter = get_additional_semantic_info(s, var, &semantic, *row_iter, input_clip_size); - if (semantic.start_row >= 0) { - for (unsigned i = 0; i < semantic.rows; ++i) - mod->input_mappings[semantic.start_row + i] = num_inputs; - } - + mod->input_mappings[var->data.driver_location] = num_inputs; mod->inputs[num_inputs].name = ralloc_strdup(mod->ralloc_ctx, semantic.name); mod->inputs[num_inputs].num_elements = semantic.rows;