[Mesa-dev] [PATCH] glsl: reduce buffer block duplication
Timothy Arceri
timothy.arceri at collabora.com
Thu Mar 24 01:11:01 UTC 2016
This reduces some of the craziness required for handing buffer
blocks. The problem is each shader stage holds its own information
about a block in memory, we were copying that information to a
program wide list but the per stage information remained meaning
when a binding was updated we needed to update all versions of it.
This changes the per stage blocks to instead points to a single
version of the block information in the program list.
---
I did try to free the per stage blocks but with some piglit/dEQP tests
it always tried to free it twice and crashed not sure what was going on.
src/compiler/glsl/link_uniform_initializers.cpp | 2 +-
src/compiler/glsl/link_uniforms.cpp | 12 ++--
src/compiler/glsl/linker.cpp | 78 +++++++++++++++----------
src/compiler/glsl/standalone_scaffolding.cpp | 5 --
src/mesa/main/mtypes.h | 9 +--
src/mesa/main/uniforms.c | 33 +----------
6 files changed, 57 insertions(+), 82 deletions(-)
diff --git a/src/compiler/glsl/link_uniform_initializers.cpp b/src/compiler/glsl/link_uniform_initializers.cpp
index 3609f81..7d280cc 100644
--- a/src/compiler/glsl/link_uniform_initializers.cpp
+++ b/src/compiler/glsl/link_uniform_initializers.cpp
@@ -183,7 +183,7 @@ set_block_binding(gl_shader_program *prog, const char *block_name, int binding)
if (stage_index != -1) {
struct gl_shader *sh = prog->_LinkedShaders[i];
- sh->BufferInterfaceBlocks[stage_index].Binding = binding;
+ sh->BufferInterfaceBlocks[stage_index]->Binding = binding;
}
}
}
diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp
index 2a8a11c..09322c5 100644
--- a/src/compiler/glsl/link_uniforms.cpp
+++ b/src/compiler/glsl/link_uniforms.cpp
@@ -954,6 +954,8 @@ link_cross_validate_uniform_block(void *mem_ctx,
new_block->Uniforms,
sizeof(*linked_block->Uniforms) * linked_block->NumUniforms);
+ linked_block->Name = ralloc_strdup(*linked_blocks, linked_block->Name);
+
for (unsigned int i = 0; i < linked_block->NumUniforms; i++) {
struct gl_uniform_buffer_variable *ubo_var =
&linked_block->Uniforms[i];
@@ -1005,9 +1007,9 @@ link_update_uniform_buffer_variables(struct gl_shader *shader)
const unsigned l = strlen(var->name);
for (unsigned i = 0; i < shader->NumBufferInterfaceBlocks; i++) {
- for (unsigned j = 0; j < shader->BufferInterfaceBlocks[i].NumUniforms; j++) {
+ for (unsigned j = 0; j < shader->BufferInterfaceBlocks[i]->NumUniforms; j++) {
if (sentinel) {
- const char *begin = shader->BufferInterfaceBlocks[i].Uniforms[j].Name;
+ const char *begin = shader->BufferInterfaceBlocks[i]->Uniforms[j].Name;
const char *end = strchr(begin, sentinel);
if (end == NULL)
@@ -1022,7 +1024,7 @@ link_update_uniform_buffer_variables(struct gl_shader *shader)
break;
}
} else if (!strcmp(var->name,
- shader->BufferInterfaceBlocks[i].Uniforms[j].Name)) {
+ shader->BufferInterfaceBlocks[i]->Uniforms[j].Name)) {
found = true;
var->data.location = j;
break;
@@ -1148,9 +1150,9 @@ link_assign_uniform_locations(struct gl_shader_program *prog,
sh->num_combined_uniform_components = sh->num_uniform_components;
for (unsigned i = 0; i < sh->NumBufferInterfaceBlocks; i++) {
- if (!sh->BufferInterfaceBlocks[i].IsShaderStorage) {
+ if (!sh->BufferInterfaceBlocks[i]->IsShaderStorage) {
sh->num_combined_uniform_components +=
- sh->BufferInterfaceBlocks[i].UniformBufferSize / 4;
+ sh->BufferInterfaceBlocks[i]->UniformBufferSize / 4;
}
}
}
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index aa2b008..6e94565 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1195,11 +1195,11 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog)
int index = link_cross_validate_uniform_block(prog,
&prog->BufferInterfaceBlocks,
&prog->NumBufferInterfaceBlocks,
- &sh->BufferInterfaceBlocks[j]);
+ sh->BufferInterfaceBlocks[j]);
if (index == -1) {
linker_error(prog, "uniform block `%s' has mismatching definitions\n",
- sh->BufferInterfaceBlocks[j].Name);
+ sh->BufferInterfaceBlocks[j]->Name);
return false;
}
@@ -1207,6 +1207,23 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog)
}
}
+ /* Update per stage block pointers to point to the program list.
+ * FIXME: We should be able to free the per stage blocks here.
+ */
+ for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
+ for (unsigned j = 0; j < prog->NumBufferInterfaceBlocks; j++) {
+ int stage_index =
+ prog->InterfaceBlockStageIndex[i][j];
+
+ if (stage_index != -1) {
+ struct gl_shader *sh = prog->_LinkedShaders[i];
+
+ sh->BufferInterfaceBlocks[stage_index] =
+ &prog->BufferInterfaceBlocks[j];
+ }
+ }
+ }
+
return true;
}
@@ -2072,9 +2089,15 @@ link_intrastage_shaders(void *mem_ctx,
linked->ir = new(linked) exec_list;
clone_ir_list(mem_ctx, linked->ir, main->ir);
- linked->BufferInterfaceBlocks = uniform_blocks;
+ linked->BufferInterfaceBlocks =
+ ralloc_array(linked, gl_uniform_block *, num_uniform_blocks);
+
+ ralloc_steal(linked, uniform_blocks);
+ for (unsigned i = 0; i < num_uniform_blocks; i++) {
+ linked->BufferInterfaceBlocks[i] = &uniform_blocks[i];
+ }
+
linked->NumBufferInterfaceBlocks = num_uniform_blocks;
- ralloc_steal(linked, linked->BufferInterfaceBlocks);
link_fs_input_layout_qualifiers(prog, linked, shader_list, num_shaders);
link_tcs_out_layout_qualifiers(prog, linked, shader_list, num_shaders);
@@ -2872,7 +2895,8 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
if (prog->InterfaceBlockStageIndex[j][i] != -1) {
struct gl_shader *sh = prog->_LinkedShaders[j];
int stage_index = prog->InterfaceBlockStageIndex[j][i];
- if (sh && sh->BufferInterfaceBlocks[stage_index].IsShaderStorage) {
+ if (sh &&
+ sh->BufferInterfaceBlocks[stage_index]->IsShaderStorage) {
shader_blocks[j]++;
total_shader_storage_blocks++;
} else {
@@ -2989,7 +3013,8 @@ check_image_resources(struct gl_context *ctx, struct gl_shader_program *prog)
for (unsigned j = 0; j < prog->NumBufferInterfaceBlocks; j++) {
int stage_index = prog->InterfaceBlockStageIndex[i][j];
- if (stage_index != -1 && sh->BufferInterfaceBlocks[stage_index].IsShaderStorage)
+ if (stage_index != -1 &&
+ sh->BufferInterfaceBlocks[stage_index]->IsShaderStorage)
total_shader_storage_blocks++;
}
@@ -4009,20 +4034,22 @@ link_assign_subroutine_types(struct gl_shader_program *prog)
static void
split_ubos_and_ssbos(void *mem_ctx,
- struct gl_uniform_block *blocks,
+ struct gl_uniform_block **s_blks,
+ struct gl_uniform_block *p_blks,
unsigned num_blocks,
struct gl_uniform_block ***ubos,
unsigned *num_ubos,
- unsigned **ubo_interface_block_indices,
struct gl_uniform_block ***ssbos,
- unsigned *num_ssbos,
- unsigned **ssbo_interface_block_indices)
+ unsigned *num_ssbos)
{
unsigned num_ubo_blocks = 0;
unsigned num_ssbo_blocks = 0;
+ /* Are we spliting the list of blocks for the shader or the program */
+ bool is_shader = p_blks == NULL;
+
for (unsigned i = 0; i < num_blocks; i++) {
- if (blocks[i].IsShaderStorage)
+ if (is_shader ? s_blks[i]->IsShaderStorage : p_blks[i].IsShaderStorage)
num_ssbo_blocks++;
else
num_ubo_blocks++;
@@ -4034,24 +4061,13 @@ split_ubos_and_ssbos(void *mem_ctx,
*ssbos = ralloc_array(mem_ctx, gl_uniform_block *, num_ssbo_blocks);
*num_ssbos = 0;
- if (ubo_interface_block_indices)
- *ubo_interface_block_indices =
- ralloc_array(mem_ctx, unsigned, num_ubo_blocks);
-
- if (ssbo_interface_block_indices)
- *ssbo_interface_block_indices =
- ralloc_array(mem_ctx, unsigned, num_ssbo_blocks);
-
for (unsigned i = 0; i < num_blocks; i++) {
- if (blocks[i].IsShaderStorage) {
- (*ssbos)[*num_ssbos] = &blocks[i];
- if (ssbo_interface_block_indices)
- (*ssbo_interface_block_indices)[*num_ssbos] = i;
+ struct gl_uniform_block *blk = is_shader ? s_blks[i] : &p_blks[i];
+ if (blk->IsShaderStorage) {
+ (*ssbos)[*num_ssbos] = blk;
(*num_ssbos)++;
} else {
- (*ubos)[*num_ubos] = &blocks[i];
- if (ubo_interface_block_indices)
- (*ubo_interface_block_indices)[*num_ubos] = i;
+ (*ubos)[*num_ubos] = blk;
(*num_ubos)++;
}
}
@@ -4645,25 +4661,23 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog,
gl_shader *sh = prog->_LinkedShaders[i];
split_ubos_and_ssbos(sh,
sh->BufferInterfaceBlocks,
+ NULL,
sh->NumBufferInterfaceBlocks,
&sh->UniformBlocks,
&sh->NumUniformBlocks,
- NULL,
&sh->ShaderStorageBlocks,
- &sh->NumShaderStorageBlocks,
- NULL);
+ &sh->NumShaderStorageBlocks);
}
}
split_ubos_and_ssbos(prog,
+ NULL,
prog->BufferInterfaceBlocks,
prog->NumBufferInterfaceBlocks,
&prog->UniformBlocks,
&prog->NumUniformBlocks,
- &prog->UboInterfaceBlockIndex,
&prog->ShaderStorageBlocks,
- &prog->NumShaderStorageBlocks,
- &prog->SsboInterfaceBlockIndex);
+ &prog->NumShaderStorageBlocks);
for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
if (prog->_LinkedShaders[i] == NULL)
diff --git a/src/compiler/glsl/standalone_scaffolding.cpp b/src/compiler/glsl/standalone_scaffolding.cpp
index df588cf..c73d506 100644
--- a/src/compiler/glsl/standalone_scaffolding.cpp
+++ b/src/compiler/glsl/standalone_scaffolding.cpp
@@ -136,11 +136,6 @@ _mesa_clear_shader_program_data(struct gl_shader_program *shProg)
shProg->InterfaceBlockStageIndex[i] = NULL;
}
- ralloc_free(shProg->UboInterfaceBlockIndex);
- shProg->UboInterfaceBlockIndex = NULL;
- ralloc_free(shProg->SsboInterfaceBlockIndex);
- shProg->SsboInterfaceBlockIndex = NULL;
-
ralloc_free(shProg->AtomicBuffers);
shProg->AtomicBuffers = NULL;
shProg->NumAtomicBuffers = 0;
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 3029760..02d9a8e 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2308,7 +2308,7 @@ struct gl_shader
* duplicated.
*/
unsigned NumBufferInterfaceBlocks;
- struct gl_uniform_block *BufferInterfaceBlocks;
+ struct gl_uniform_block **BufferInterfaceBlocks;
unsigned NumUniformBlocks;
struct gl_uniform_block **UniformBlocks;
@@ -2826,13 +2826,6 @@ struct gl_shader_program
int *InterfaceBlockStageIndex[MESA_SHADER_STAGES];
/**
- * Indices into the BufferInterfaceBlocks[] array for Uniform Buffer
- * Objects and Shader Storage Buffer Objects.
- */
- unsigned *UboInterfaceBlockIndex;
- unsigned *SsboInterfaceBlockIndex;
-
- /**
* Map of active uniform names to locations
*
* Maps any active uniform that is not an array element to a location.
diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
index b1968b3..7dcbdcc 100644
--- a/src/mesa/main/uniforms.c
+++ b/src/mesa/main/uniforms.c
@@ -1018,26 +1018,11 @@ _mesa_UniformBlockBinding(GLuint program,
if (shProg->UniformBlocks[uniformBlockIndex]->Binding !=
uniformBlockBinding) {
- int i;
FLUSH_VERTICES(ctx, 0);
ctx->NewDriverState |= ctx->DriverFlags.NewUniformBuffer;
- const int interface_block_index =
- shProg->UboInterfaceBlockIndex[uniformBlockIndex];
-
- shProg->BufferInterfaceBlocks[interface_block_index].Binding =
- uniformBlockBinding;
-
- for (i = 0; i < MESA_SHADER_STAGES; i++) {
- int stage_index =
- shProg->InterfaceBlockStageIndex[i][interface_block_index];
-
- if (stage_index != -1) {
- struct gl_shader *sh = shProg->_LinkedShaders[i];
- sh->BufferInterfaceBlocks[stage_index].Binding = uniformBlockBinding;
- }
- }
+ shProg->UniformBlocks[uniformBlockIndex]->Binding = uniformBlockBinding;
}
}
@@ -1076,26 +1061,12 @@ _mesa_ShaderStorageBlockBinding(GLuint program,
if (shProg->ShaderStorageBlocks[shaderStorageBlockIndex]->Binding !=
shaderStorageBlockBinding) {
- int i;
FLUSH_VERTICES(ctx, 0);
ctx->NewDriverState |= ctx->DriverFlags.NewShaderStorageBuffer;
- const int interface_block_index =
- shProg->SsboInterfaceBlockIndex[shaderStorageBlockIndex];
-
- shProg->BufferInterfaceBlocks[interface_block_index].Binding =
+ shProg->ShaderStorageBlocks[shaderStorageBlockIndex]->Binding =
shaderStorageBlockBinding;
-
- for (i = 0; i < MESA_SHADER_STAGES; i++) {
- int stage_index =
- shProg->InterfaceBlockStageIndex[i][interface_block_index];
-
- if (stage_index != -1) {
- struct gl_shader *sh = shProg->_LinkedShaders[i];
- sh->BufferInterfaceBlocks[stage_index].Binding = shaderStorageBlockBinding;
- }
- }
}
}
--
2.5.5
More information about the mesa-dev
mailing list