From e941732ab19d775b1654a3881961e68c61300293 Mon Sep 17 00:00:00 2001 From: Iago Toral Quiroga Date: Thu, 27 Jul 2023 06:53:44 +0200 Subject: [PATCH] v3dv: stop incrementing UBO indices by one MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This matches what we do for OpenGL, allowing us to have the same compiler behavior for both worlds. Reviewed-by: Alejandro Piñeiro Part-of: --- src/broadcom/compiler/nir_to_vir.c | 29 +++++++++++------------------ src/broadcom/compiler/vir.c | 1 - src/broadcom/vulkan/v3dv_pipeline.c | 10 +--------- src/broadcom/vulkan/v3dv_uniforms.c | 7 ++++--- 4 files changed, 16 insertions(+), 31 deletions(-) diff --git a/src/broadcom/compiler/nir_to_vir.c b/src/broadcom/compiler/nir_to_vir.c index 4472af7..af0549a 100644 --- a/src/broadcom/compiler/nir_to_vir.c +++ b/src/broadcom/compiler/nir_to_vir.c @@ -564,13 +564,11 @@ ntq_emit_tmu_general(struct v3d_compile *c, nir_intrinsic_instr *instr, v3d_unit_data_create(0, const_offset)); const_offset = 0; } else if (instr->intrinsic == nir_intrinsic_load_ubo) { - uint32_t index = nir_src_as_uint(instr->src[0]); - /* On OpenGL QUNIFORM_UBO_ADDR takes a UBO index - * shifted up by 1 (0 is gallium's constant buffer 0). + /* QUNIFORM_UBO_ADDR takes a UBO index shifted up by 1 (0 + * is gallium's constant buffer 0 in GL and push constants + * in Vulkan)). */ - if (c->key->environment == V3D_ENVIRONMENT_OPENGL) - index++; - + uint32_t index = nir_src_as_uint(instr->src[0]) + 1; base_offset = vir_uniform(c, QUNIFORM_UBO_ADDR, v3d_unit_data_create(index, const_offset)); @@ -2741,13 +2739,10 @@ ntq_emit_inline_ubo_load(struct v3d_compile *c, nir_intrinsic_instr *instr) if (c->compiler->max_inline_uniform_buffers <= 0) return false; - /* On Vulkan we use indices 1..MAX_INLINE_UNIFORM_BUFFERS for inline - * uniform buffers which we want to handle more like push constants - * than regular UBO. OpenGL doesn't implement this feature. - */ assert(c->key->environment == V3D_ENVIRONMENT_VULKAN); + /* Regular UBOs start after inline UBOs */ uint32_t index = nir_src_as_uint(instr->src[0]); - if (index == 0 || index > c->compiler->max_inline_uniform_buffers) + if (index >= c->compiler->max_inline_uniform_buffers) return false; /* We scalarize general TMU access for anything that is not 32-bit */ @@ -2755,13 +2750,10 @@ ntq_emit_inline_ubo_load(struct v3d_compile *c, nir_intrinsic_instr *instr) instr->num_components == 1); if (nir_src_is_const(instr->src[1])) { - /* Index 0 is reserved for push constants */ - assert(index > 0); - uint32_t inline_index = index - 1; int offset = nir_src_as_uint(instr->src[1]); if (try_emit_uniform(c, offset, instr->num_components, &instr->dest, - QUNIFORM_INLINE_UBO_0 + inline_index)) { + QUNIFORM_INLINE_UBO_0 + index)) { return true; } } @@ -3159,10 +3151,11 @@ ntq_emit_load_unifa(struct v3d_compile *c, nir_intrinsic_instr *instr) */ uint32_t index = is_uniform ? 0 : nir_src_as_uint(instr->src[0]); - /* On OpenGL QUNIFORM_UBO_ADDR takes a UBO index - * shifted up by 1 (0 is gallium's constant buffer 0). + /* QUNIFORM_UBO_ADDR takes a UBO index shifted up by 1 since we use + * index 0 for Gallium's constant buffer (GL) or push constants + * (Vulkan). */ - if (is_ubo && c->key->environment == V3D_ENVIRONMENT_OPENGL) + if (is_ubo) index++; /* We can only keep track of the last unifa address we used with diff --git a/src/broadcom/compiler/vir.c b/src/broadcom/compiler/vir.c index ec0dca7..2fa4b25 100644 --- a/src/broadcom/compiler/vir.c +++ b/src/broadcom/compiler/vir.c @@ -1603,7 +1603,6 @@ v3d_attempt_compile(struct v3d_compile *c) .lower_image = c->key->robust_image_access, .lower_ssbo = c->key->robust_storage_access, .lower_ubo = c->key->robust_uniform_access, - .skip_ubo_0 = c->key->environment == V3D_ENVIRONMENT_VULKAN, }; NIR_PASS(_, c->s, nir_lower_robust_access, &opts); diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c index 99fe8c1..1518d13 100644 --- a/src/broadcom/vulkan/v3dv_pipeline.c +++ b/src/broadcom/vulkan/v3dv_pipeline.c @@ -546,7 +546,7 @@ lower_vulkan_resource_index(nir_builder *b, uint32_t start_index = 0; if (binding_layout->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER || binding_layout->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC) { - start_index = MAX_INLINE_UNIFORM_BUFFERS; + start_index += MAX_INLINE_UNIFORM_BUFFERS; } index = descriptor_map_add(descriptor_map, set, binding, @@ -555,14 +555,6 @@ lower_vulkan_resource_index(nir_builder *b, start_index, 32 /* return_size: doesn't really apply for this case */, 0); - - /* We always reserve index 0 for push constants */ - if (binding_layout->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER || - binding_layout->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC || - binding_layout->type == VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK) { - index++; - } - break; } diff --git a/src/broadcom/vulkan/v3dv_uniforms.c b/src/broadcom/vulkan/v3dv_uniforms.c index 72fa9a1..2d3f017 100644 --- a/src/broadcom/vulkan/v3dv_uniforms.c +++ b/src/broadcom/vulkan/v3dv_uniforms.c @@ -288,9 +288,10 @@ write_ubo_ssbo_uniforms(struct v3dv_cmd_buffer *cmd_buffer, offset + dynamic_offset); } else { if (content == QUNIFORM_UBO_ADDR) { - /* We reserve index 0 for push constants and artificially increase our - * indices by one for that reason, fix that now before accessing the - * descriptor map. + /* We reserve UBO index 0 for push constants in Vulkan (and for the + * constant buffer in GL) so the compiler always adds one to all UBO + * indices, fix it up before we access the descriptor map, since + * indices start from 0 there. */ assert(index > 0); index--; -- 2.7.4