Re: [Mesa-dev] [PATCH 15/21] linker: Modify cross_validate_outputs_to_inputs to match using explicit locations
Ian Romanick i...@freedesktop.org writes: From: Ian Romanick ian.d.roman...@intel.com This will be used for GL_ARB_separate_shader_objects. That extension not only allows separable shaders to rendezvous by location, but it also allows traditionally linked shaders to rendezvous by location. The spec says: 36. How does the behavior of input/output interface matching differ between separable programs and non-separable programs? RESOLVED: The rules for matching individual variables or block members between stages are identical for separable and non-separable programs, with one exception -- matching variables of different type with the same location, as discussed in issue 34, applies only to separable programs. However, the ability to enforce matching requirements differs between program types. In non-separable programs, both sides of an interface are contained in the same linked program. In this case, if the linker detects a mismatch, it will generate a link error. diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp index e5c2751..232e230 100644 --- a/src/glsl/link_varyings.cpp +++ b/src/glsl/link_varyings.cpp @@ -172,6 +172,7 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog, gl_shader *producer, gl_shader *consumer) { glsl_symbol_table parameters; + ir_variable *explicit_locations[MAX_VARYING] = { NULL, }; /* Find all shader outputs in the producer stage. */ @@ -181,7 +182,26 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog, if ((var == NULL) || (var-data.mode != ir_var_shader_out)) continue; - parameters.add_variable(var); + if (!var-data.explicit_location + || var-data.location VARYING_SLOT_VAR0) + parameters.add_variable(var); + else { + /* User-defined varyings with explicit locations are handled + * differently because they do not need to have matching names. + */ + const unsigned idx = var-data.location - VARYING_SLOT_VAR0; + + if (explicit_locations[idx] != NULL) { +linker_error(prog, + %s shader has multiple outputs explicitly + assigned to location %d\n, + _mesa_shader_stage_to_string(producer-Stage), + idx); +return; Relevant citation: A program will fail to link if any two fragment shader output variables are assigned to the same location and index, or if any two output variables from the same non-fragment shader stage are assigned to the same location. I think this has a problem of not throwing an error when your explicit location array overlaps your explicit location scalar, but I don't think it's that important. @@ -1051,8 +1091,13 @@ namespace linker { bool populate_consumer_input_sets(void *mem_ctx, exec_list *ir, hash_table *consumer_inputs, - hash_table *consumer_interface_inputs) + hash_table *consumer_interface_inputs, + ir_variable *consumer_inputs_with_locations[MAX_VARYING]) { + memset(consumer_inputs_with_locations, + 0, + sizeof(consumer_inputs_with_locations[0]) * MAX_VARYING); + foreach_list(node, ir) { ir_variable *const input_var = ((ir_instruction *) node)-as_variable(); @@ -1060,7 +1105,13 @@ populate_consumer_input_sets(void *mem_ctx, exec_list *ir, if (input_var-type-is_interface()) return false; - if (input_var-get_interface_type() != NULL) { + if (input_var-data.explicit_location) { +/* FINISHME: If the input is an interface, array, or structure, it + * FINISHME: will use multiple slots. + */ +consumer_inputs_with_locations[input_var-data.location] = + input_var; I think this FINISHME shouldn't be there. It looks like assign_varying_locations() only cares about finding the ir_variable at the start of a contiguous location block. My reasoning: For !producer, consumer_inputs_with_locations isn't used. For !consumer, consumer_inputs_with_locations is empty. For consumer producer, if you were trying to set some ir_variable to the middle of a location block on the other side of producer/consumer, cross_validate_outputs_to_inputs() should be link-erroring due to either type mismatch or location overlaps. If the variables do match up, then they've got a matching data.location and you only looked at consumer_inputs_with_locations[var-data.location], not any following entries for the array/structure. pgp1J3otjuldz.pgp Description: PGP signature ___ mesa-dev mailing
Re: [Mesa-dev] [PATCH 15/21] linker: Modify cross_validate_outputs_to_inputs to match using explicit locations
On 04/30/2014 12:40 PM, Eric Anholt wrote: Ian Romanick i...@freedesktop.org writes: From: Ian Romanick ian.d.roman...@intel.com This will be used for GL_ARB_separate_shader_objects. That extension not only allows separable shaders to rendezvous by location, but it also allows traditionally linked shaders to rendezvous by location. The spec says: 36. How does the behavior of input/output interface matching differ between separable programs and non-separable programs? RESOLVED: The rules for matching individual variables or block members between stages are identical for separable and non-separable programs, with one exception -- matching variables of different type with the same location, as discussed in issue 34, applies only to separable programs. However, the ability to enforce matching requirements differs between program types. In non-separable programs, both sides of an interface are contained in the same linked program. In this case, if the linker detects a mismatch, it will generate a link error. diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp index e5c2751..232e230 100644 --- a/src/glsl/link_varyings.cpp +++ b/src/glsl/link_varyings.cpp @@ -172,6 +172,7 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog, gl_shader *producer, gl_shader *consumer) { glsl_symbol_table parameters; + ir_variable *explicit_locations[MAX_VARYING] = { NULL, }; /* Find all shader outputs in the producer stage. */ @@ -181,7 +182,26 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog, if ((var == NULL) || (var-data.mode != ir_var_shader_out)) continue; - parameters.add_variable(var); + if (!var-data.explicit_location + || var-data.location VARYING_SLOT_VAR0) + parameters.add_variable(var); + else { + /* User-defined varyings with explicit locations are handled + * differently because they do not need to have matching names. + */ + const unsigned idx = var-data.location - VARYING_SLOT_VAR0; + + if (explicit_locations[idx] != NULL) { +linker_error(prog, + %s shader has multiple outputs explicitly + assigned to location %d\n, + _mesa_shader_stage_to_string(producer-Stage), + idx); +return; Relevant citation: A program will fail to link if any two fragment shader output variables are assigned to the same location and index, or if any two output variables from the same non-fragment shader stage are assigned to the same location. I think this has a problem of not throwing an error when your explicit location array overlaps your explicit location scalar, but I don't think it's that important. I think you're correct on both counts. I'm not sure if the closed-source drivers enforce this either. More tests. @@ -1051,8 +1091,13 @@ namespace linker { bool populate_consumer_input_sets(void *mem_ctx, exec_list *ir, hash_table *consumer_inputs, - hash_table *consumer_interface_inputs) + hash_table *consumer_interface_inputs, + ir_variable *consumer_inputs_with_locations[MAX_VARYING]) { + memset(consumer_inputs_with_locations, + 0, + sizeof(consumer_inputs_with_locations[0]) * MAX_VARYING); + foreach_list(node, ir) { ir_variable *const input_var = ((ir_instruction *) node)-as_variable(); @@ -1060,7 +1105,13 @@ populate_consumer_input_sets(void *mem_ctx, exec_list *ir, if (input_var-type-is_interface()) return false; - if (input_var-get_interface_type() != NULL) { + if (input_var-data.explicit_location) { +/* FINISHME: If the input is an interface, array, or structure, it + * FINISHME: will use multiple slots. + */ +consumer_inputs_with_locations[input_var-data.location] = + input_var; I think this FINISHME shouldn't be there. It looks like assign_varying_locations() only cares about finding the ir_variable at the start of a contiguous location block. My reasoning: For !producer, consumer_inputs_with_locations isn't used. For !consumer, consumer_inputs_with_locations is empty. For consumer producer, if you were trying to set some ir_variable to the middle of a location block on the other side of producer/consumer, cross_validate_outputs_to_inputs() should be link-erroring due to either type mismatch or location overlaps. If the variables do match up, then they've got a matching data.location and you only looked at