Re: [Mesa-dev] [PATCH 2/3] clover: Add all the mandatory 1.1 extensions to the extension string
On Sat, 2015-03-21 at 00:35 +, Tom Stellard wrote: --- src/gallium/state_trackers/clover/api/device.cpp | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp index 04f293d..43e7475 100644 --- a/src/gallium/state_trackers/clover/api/device.cpp +++ b/src/gallium/state_trackers/clover/api/device.cpp @@ -301,7 +301,13 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param, break; case CL_DEVICE_EXTENSIONS: - buf.as_string() = dev.has_doubles() ? cl_khr_fp64 : ; + buf.as_string() = + cl_khr_global_int32_base_atomics + cl_khr_global_int32_extended_atomics + cl_khr_local_int32_base_atomics + cl_khr_local_int32_extended_atomics + cl_khr_byte_addressable_store + + std::string(dev.has_doubles() ? cl_khr_fp64 : ); break; case CL_DEVICE_PLATFORM: Reviewed-by: Jan Vesely jan.ves...@rutgers.edu -- Jan Vesely jan.ves...@rutgers.edu signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: The unit for CL_DEVICE_MEM_BASE_ADDR_ALIGN is bits not bytes
On Sat, 2015-03-21 at 02:08 +, Tom Stellard wrote: --- src/gallium/state_trackers/clover/api/device.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp index 43e7475..bc93f91 100644 --- a/src/gallium/state_trackers/clover/api/device.cpp +++ b/src/gallium/state_trackers/clover/api/device.cpp @@ -196,6 +196,9 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param, break; case CL_DEVICE_MEM_BASE_ADDR_ALIGN: + buf.as_scalarcl_uint() = 128 * 8; + break; + case CL_DEVICE_MIN_DATA_TYPE_ALIGN_SIZE: buf.as_scalarcl_uint() = 128; break; Reviewed-by: Jan Vesely jan.ves...@rutgers.edu -- Jan Vesely jan.ves...@rutgers.edu signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 10/12] glsl: add AoA support for an inteface with unsized array members
Add support for setting the max access of an unsized member of an interface array of arrays. For example ifc[j][k].foo[i] where foo is unsized. --- src/glsl/ast_array_index.cpp | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp index ecef651..481bba8 100644 --- a/src/glsl/ast_array_index.cpp +++ b/src/glsl/ast_array_index.cpp @@ -64,21 +64,29 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc, } } else if (ir_dereference_record *deref_record = ir-as_dereference_record()) { - /* There are two possibilities we need to consider: + /* There are three possibilities we need to consider: * * - Accessing an element of an array that is a member of a named * interface block (e.g. ifc.foo[i]) * * - Accessing an element of an array that is a member of a named * interface block array (e.g. ifc[j].foo[i]). + * + * - Accessing an element of an array that is a member of a named + * interface block array of arrays (e.g. ifc[j][k].foo[i]). */ ir_dereference_variable *deref_var = deref_record-record-as_dereference_variable(); if (deref_var == NULL) { - if (ir_dereference_array *deref_array = - deref_record-record-as_dereference_array()) { -deref_var = deref_array-array-as_dereference_variable(); + ir_dereference_array *deref_array = +deref_record-record-as_dereference_array(); + ir_dereference_array *deref_array_prev = NULL; + while (deref_array != NULL) { +deref_array_prev = deref_array; +deref_array = deref_array-array-as_dereference_array(); } + if (deref_array_prev != NULL) +deref_var = deref_array_prev-array-as_dereference_variable(); } if (deref_var != NULL) { -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 07/12] glsl: Add support for linking uniform arrays of arrays
--- src/glsl/link_uniform_initializers.cpp | 51 -- src/glsl/link_uniforms.cpp | 28 +++ 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp index 6907384..7817314 100644 --- a/src/glsl/link_uniform_initializers.cpp +++ b/src/glsl/link_uniform_initializers.cpp @@ -100,6 +100,38 @@ copy_constant_to_storage(union gl_constant_value *storage, } void +copy_constant_array_to_storage(struct gl_uniform_storage *const storage, + const ir_constant *val, + unsigned int *idx, + unsigned int *array_elements, + unsigned int boolean_true) +{ + if (val-type-fields.array-is_array()) { + for (unsigned int i = 0; i val-type-length; i++) { + copy_constant_array_to_storage(storage, val-array_elements[i], +idx, array_elements, boolean_true); + } + } else { + const enum glsl_base_type base_type = +val-array_elements[0]-type-base_type; + const unsigned int elements = val-array_elements[0]-type-components(); + unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int length = MIN2(val-type-length, + (storage-array_elements - *array_elements)); + + for (unsigned int i = 0; i length; i++) { + copy_constant_to_storage( storage-storage[*idx], + val-array_elements[i], + base_type, + elements, + boolean_true); + *idx += elements * dmul; + *array_elements = *array_elements + 1; + } + } +} + +void set_sampler_binding(gl_shader_program *prog, const char *name, int binding) { struct gl_uniform_storage *const storage = @@ -178,7 +210,7 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, field_constant = (ir_constant *)field_constant-next; } return; - } else if (type-is_array() type-fields.array-is_record()) { + } else if (type-without_array()-is_record()) { const glsl_type *const element_type = type-fields.array; for (unsigned int i = 0; i type-length; i++) { @@ -201,22 +233,11 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, } if (val-type-is_array()) { - const enum glsl_base_type base_type = -val-array_elements[0]-type-base_type; - const unsigned int elements = val-array_elements[0]-type-components(); unsigned int idx = 0; - unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int array_elements = 0; - assert(val-type-length = storage-array_elements); - for (unsigned int i = 0; i storage-array_elements; i++) { -copy_constant_to_storage( storage-storage[idx], - val-array_elements[i], - base_type, - elements, - boolean_true); - -idx += elements * dmul; - } + copy_constant_array_to_storage(storage, val, idx, + array_elements, boolean_true); } else { copy_constant_to_storage(storage-storage, val, diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp index 799c74b..654a1ca 100644 --- a/src/glsl/link_uniforms.cpp +++ b/src/glsl/link_uniforms.cpp @@ -49,8 +49,8 @@ values_for_type(const glsl_type *type) { if (type-is_sampler()) { return 1; - } else if (type-is_array() type-fields.array-is_sampler()) { - return type-array_size(); + } else if (type-is_array()) { + return type-array_size() * values_for_type(type-fields.array); } else { return type-component_slots(); } @@ -71,6 +71,7 @@ void program_resource_visitor::process(ir_variable *var) { const glsl_type *t = var-type; + const glsl_type *t_without_array = var-type-without_array(); const bool row_major = var-data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR; @@ -136,7 +137,7 @@ program_resource_visitor::process(ir_variable *var) */ recursion(var-type, name, strlen(name), row_major, NULL, false); ralloc_free(name); - } else if (t-without_array()-is_record()) { + } else if (t_without_array-is_record()) { char *name = ralloc_strdup(NULL, var-name); recursion(var-type, name, strlen(name), row_major, NULL, false); ralloc_free(name); @@ -144,8 +145,8 @@ program_resource_visitor::process(ir_variable *var) char *name = ralloc_strdup(NULL, var-type-name); recursion(var-type, name, strlen(name), row_major, NULL, false); ralloc_free(name); - } else if (t-is_array() t-fields.array-is_interface()) { -
[Mesa-dev] [RFC PATCH 02/12] glsl: add arrays of arrays support to without_array function
Signed-off-by: Timothy Arceri t_arc...@yahoo.com.au --- src/glsl/glsl_types.h | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h index 7359e94..f5bc846 100644 --- a/src/glsl/glsl_types.h +++ b/src/glsl/glsl_types.h @@ -534,7 +534,12 @@ struct glsl_type { */ const glsl_type *without_array() const { - return this-is_array() ? this-fields.array : this; + const glsl_type *t = this; + + while (t-is_array()) + t = t-fields.array; + + return t; } /** -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 08/12] glsl: update assert to support arrays of arrays
--- src/glsl/glsl_types.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp index 38b37a6..994c5c5 100644 --- a/src/glsl/glsl_types.cpp +++ b/src/glsl/glsl_types.cpp @@ -1016,7 +1016,8 @@ glsl_type::std140_base_alignment(bool row_major) const this-fields.array-is_matrix()) { return MAX2(this-fields.array-std140_base_alignment(row_major), 16); } else { -assert(this-fields.array-is_record()); +assert(this-fields.array-is_record() || +this-fields.array-is_array()); return this-fields.array-std140_base_alignment(row_major); } } -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 09/12] glsl: add AoA support for linking interface blocks
Adds support for linking AoA interface blocks which countain unsized arrays --- src/glsl/ir.cpp | 4 ++-- src/glsl/linker.cpp | 9 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp index 54656f8..2e1442f 100644 --- a/src/glsl/ir.cpp +++ b/src/glsl/ir.cpp @@ -1663,8 +1663,8 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name, if (type-is_interface()) this-init_interface_type(type); - else if (type-is_array() type-fields.array-is_interface()) - this-init_interface_type(type-fields.array); + else if (type-without_array()-is_interface()) + this-init_interface_type(type-without_array()); } } diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 0c44677..eb25163 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -1201,7 +1201,9 @@ public: virtual ir_visitor_status visit(ir_variable *var) { + const glsl_type *type_without_array; fixup_type(var-type, var-data.max_array_access); + type_without_array = var-type-without_array(); if (var-type-is_interface()) { if (interface_contains_unsized_arrays(var-type)) { const glsl_type *new_type = @@ -1210,11 +1212,10 @@ public: var-type = new_type; var-change_interface_type(new_type); } - } else if (var-type-is_array() - var-type-fields.array-is_interface()) { - if (interface_contains_unsized_arrays(var-type-fields.array)) { + } else if (type_without_array-is_interface()) { + if (interface_contains_unsized_arrays(type_without_array)) { const glsl_type *new_type = - resize_interface_members(var-type-fields.array, + resize_interface_members(type_without_array, var-get_max_ifc_array_access()); var-change_interface_type(new_type); var-type = -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 01/12] glsl: calculate component size for arrays of arrays when varying packing disabled
Signed-off-by: Timothy Arceri t_arc...@yahoo.com.au --- src/glsl/link_varyings.cpp | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp index 2261799..80c54cc 100644 --- a/src/glsl/link_varyings.cpp +++ b/src/glsl/link_varyings.cpp @@ -863,9 +863,18 @@ varying_matches::record(ir_variable *producer_var, ir_variable *consumer_var) this-matches[this-num_matches].packing_order = this-compute_packing_order(var); if (this-disable_varying_packing) { - unsigned slots = var-type-is_array() - ? (var-type-length * var-type-fields.array-matrix_columns) - : var-type-matrix_columns; + unsigned slots; + if (var-type-is_array()) { + const glsl_type *type = var-type; + slots = 1; + while (type-is_array()) { +slots *= type-length; +type = type-fields.array; + } + slots *= type-matrix_columns; + } else { + slots = var-type-matrix_columns; + } this-matches[this-num_matches].num_components = 4 * slots; } else { this-matches[this-num_matches].num_components -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 04/12] glsl: replace while loop with without_array function
Signed-off-by: Timothy Arceri t_arc...@yahoo.com.au --- src/glsl/ast_to_hir.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index d387b2e..36f3eb7 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -3584,9 +3584,7 @@ ast_declarator_list::hir(exec_list *instructions, *vectors. Vertex shader inputs cannot be arrays or *structures. */ -const glsl_type *check_type = var-type; -while (check_type-is_array()) - check_type = check_type-element_type(); +const glsl_type *check_type = var-type-without_array(); switch (check_type-base_type) { case GLSL_TYPE_FLOAT: -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 11/12] glsl: update types for unsized array members
Assigns a new array type based on the max access of unsized array members. --- src/glsl/linker.cpp | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index eb25163..97f5023 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -1218,8 +1218,7 @@ public: resize_interface_members(type_without_array, var-get_max_ifc_array_access()); var-change_interface_type(new_type); -var-type = - glsl_type::get_array_instance(new_type, var-type-length); +var-type = update_interface_members_array(var-type, new_type); } } else if (const glsl_type *ifc_type = var-get_interface_type()) { /* Store a pointer to the variable in the unnamed_interfaces @@ -1267,6 +1266,21 @@ private: } } + static const glsl_type * + update_interface_members_array(const glsl_type *type, + const glsl_type *new_interface_type) + { + const glsl_type *element_type = type-fields.array; + if (element_type-is_array()) { + const glsl_type *new_array_type = +update_interface_members_array(element_type, new_interface_type); + return glsl_type::get_array_instance(new_array_type, type-length); + } else { + return glsl_type::get_array_instance(new_interface_type, + type-length); + } + } + /** * Determine whether the given interface type contains unsized arrays (if * it doesn't, array_sizing_visitor doesn't need to process it). -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC] More ARB_arrays_of_arrays support
This series adds most of the remaining glsl arrays of arrays support. Support for uniform blocks is still missing, I've played around with this but don't have anything working yet. What do you guys think about adding these changes without the uniform blocks support? Adding these changes now I assume would make it easy for the nir guys to start adding AoA support (if it doesn't exist already) and the intel backend changes could start to be worked on. For anyone interested the main remaining changes to be made are in: ./glsl/link_uniform_block_active_visitor.cpp ./glsl/link_uniform_blocks.cpp ./glsl/lower_ubo_reference.cpp For gallium drivers the glsl to tgsi conversion seems to handle everything without changes so at least for the piglit tests I've written everything seem to just work. There are no piglit regressions from these changes (tested on i965 driver) and all the current arrays of arrays piglit test pass (tested with ilo gallium driver using MESA_GL_VERSION_OVERRIDE=3.3COMPAT MESA_GLSL_VERSION_OVERRIDE=150) besides the uniform block tests. There are also some unreviewed interface linking tests relating to the last 4 patches on the piglit list [1]. I also have some generated execution tests in my piglit repo [2], I'm not sure if I've created these the best way and wish to extend them further so these haven't been sent to the piglit list yet. [1] http://lists.freedesktop.org/archives/piglit/2015-March/015167.html [2] https://github.com/tarceri/Mesa_arrays_of_arrays_piglit/tree/execution_tests ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 12/12] glsl: Add support for lowering interface block arrays of arrays
--- src/glsl/lower_named_interface_blocks.cpp | 51 ++- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/src/glsl/lower_named_interface_blocks.cpp b/src/glsl/lower_named_interface_blocks.cpp index 7304c51..8321bc1 100644 --- a/src/glsl/lower_named_interface_blocks.cpp +++ b/src/glsl/lower_named_interface_blocks.cpp @@ -65,6 +65,38 @@ #include ir_rvalue_visitor.h #include program/hash_table.h +const glsl_type * +process_array_type(const glsl_type *type, unsigned idx) +{ + const glsl_type *element_type = type-fields.array; + if (element_type-is_array()) { + const glsl_type *new_array_type = process_array_type(element_type, idx); + return glsl_type::get_array_instance(new_array_type, type-length); + } else { + return glsl_type::get_array_instance( +element_type-fields.structure[idx].type, type-length); + } +} + +ir_rvalue * +process_array_ir(void * const mem_ctx, + ir_dereference_array *deref_array_prev, + ir_rvalue *deref_var) +{ + ir_dereference_array *deref_array = + deref_array_prev-array-as_dereference_array(); + + if (deref_array == NULL) { + return new(mem_ctx) ir_dereference_array(deref_var, + deref_array_prev-array_index); + } else { + deref_array = (ir_dereference_array *) + process_array_ir(mem_ctx, deref_array, deref_var); + return new(mem_ctx) ir_dereference_array(deref_array, + deref_array_prev-array_index); + } +} + namespace { class flatten_named_interface_blocks_declarations : public ir_rvalue_visitor @@ -111,15 +143,9 @@ flatten_named_interface_blocks_declarations::run(exec_list *instructions) if (var-data.mode == ir_var_uniform) continue; - const glsl_type * iface_t = var-type; - const glsl_type * array_t = NULL; + const glsl_type * iface_t = var-type-without_array(); exec_node *insert_pos = var; - if (iface_t-is_array()) { - array_t = iface_t; - iface_t = array_t-fields.array; - } - assert (iface_t-is_interface()); for (unsigned i = 0; i iface_t-length; i++) { @@ -135,7 +161,7 @@ flatten_named_interface_blocks_declarations::run(exec_list *instructions) ir_variable *new_var; char *var_name = ralloc_strdup(mem_ctx, iface_t-fields.structure[i].name); -if (array_t == NULL) { +if (!var-type-is_array()) { new_var = new(mem_ctx) ir_variable(iface_t-fields.structure[i].type, var_name, @@ -143,9 +169,7 @@ flatten_named_interface_blocks_declarations::run(exec_list *instructions) new_var-data.from_named_ifc_block_nonarray = 1; } else { const glsl_type *new_array_type = - glsl_type::get_array_instance( - iface_t-fields.structure[i].type, - array_t-length); + process_array_type(var-type, i); new_var = new(mem_ctx) ir_variable(new_array_type, var_name, @@ -231,9 +255,8 @@ flatten_named_interface_blocks_declarations::handle_rvalue(ir_rvalue **rvalue) ir_dereference_array *deref_array = ir-record-as_dereference_array(); if (deref_array != NULL) { - *rvalue = -new(mem_ctx) ir_dereference_array(deref_var, - deref_array-array_index); + *rvalue = process_array_ir(mem_ctx, deref_array, +(ir_rvalue *)deref_var); } else { *rvalue = deref_var; } -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 05/12] glsl: avoid hitting assert for arrays of arrays
Also add TODO comment about adding proper support Signed-off-by: Timothy Arceri t_arc...@yahoo.com.au --- src/glsl/ir_set_program_inouts.cpp | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/glsl/ir_set_program_inouts.cpp b/src/glsl/ir_set_program_inouts.cpp index e877a20..49cf85d 100644 --- a/src/glsl/ir_set_program_inouts.cpp +++ b/src/glsl/ir_set_program_inouts.cpp @@ -194,6 +194,12 @@ ir_set_program_inouts_visitor::try_mark_partial_variable(ir_variable *var, type = type-fields.array; } + /* TODO: implement proper arrays of arrays support +* for now let the caller mark whole variable as used. +*/ + if (type-is_array() type-fields.array-is_array()) + return false; + /* The code below only handles: * * - Indexing into matrices -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 03/12] glsl: support packing of arrays of arrays
--- src/glsl/lower_packed_varyings.cpp | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/glsl/lower_packed_varyings.cpp b/src/glsl/lower_packed_varyings.cpp index 2c9a1c4..f8e79bd 100644 --- a/src/glsl/lower_packed_varyings.cpp +++ b/src/glsl/lower_packed_varyings.cpp @@ -645,14 +645,8 @@ lower_packed_varyings_visitor::needs_lowering(ir_variable *var) if (var-data.explicit_location) return false; - const glsl_type *type = var-type; - if (this-gs_input_vertices != 0) { - assert(type-is_array()); - type = type-element_type(); - } - if (type-is_array()) - type = type-fields.array; - if (type-vector_elements == 4 !type-is_double()) + if (var-type-without_array()-vector_elements == 4 + !var-type-is_double()) return false; return true; } -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH 06/12] glsl: dont allow gl_PerVertex to be redeclared as an array of arrays
Signed-off-by: Timothy Arceri t_arc...@yahoo.com.au --- src/glsl/ast_to_hir.cpp | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 36f3eb7..7516e60 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -5595,7 +5595,11 @@ ast_interface_block::hir(exec_list *instructions, _mesa_shader_stage_to_string(state-stage)); } if (this-instance_name == NULL || - strcmp(this-instance_name, gl_in) != 0 || this-array_specifier == NULL) { + strcmp(this-instance_name, gl_in) != 0 || this-array_specifier == NULL || + (this-array_specifier-is_unsized_array + !this-array_specifier-array_dimensions.is_empty()) || + (this-array_specifier-array_dimensions.tail_pred-prev != NULL + !this-array_specifier-array_dimensions.tail_pred-prev-is_head_sentinel())) { _mesa_glsl_error(loc, state, gl_PerVertex input must be redeclared as gl_in[]); -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: Return 0 as storage size for local kernel args that are not set
On Fri, 2015-03-20 at 23:29 +, Tom Stellard wrote: The storage size for local kernel args can be queried before the arguments are set by using the CL_KERNEL_LOCAL_MEM_SIZE param of clGetKernelWorkGroupInfo(). The spec says that if local kernel arguments have not been specified, then we should assume their size is 0. --- src/gallium/state_trackers/clover/core/kernel.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp b/src/gallium/state_trackers/clover/core/kernel.cpp index 442762c..f4f8616 100644 --- a/src/gallium/state_trackers/clover/core/kernel.cpp +++ b/src/gallium/state_trackers/clover/core/kernel.cpp @@ -399,6 +399,8 @@ kernel::global_argument::unbind(exec_context ctx) { size_t kernel::local_argument::storage() const { + if (!_set) + return 0; Shouldn't _storage be 0 before anybody calls local_argument::set anyway? maybe initializing it in a constructor would suffice? jan return _storage; } -- Jan Vesely jan.ves...@rutgers.edu signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89689] [Regression] Weston on DRM backend won't start with new version of mesa
https://bugs.freedesktop.org/show_bug.cgi?id=89689 --- Comment #3 from Boyan Ding stu_...@126.com --- Created attachment 114500 -- https://bugs.freedesktop.org/attachment.cgi?id=114500action=edit dri/i965: Add XRGB to dri config I hacked the i965 driver and wrote this patch. After applying this to mesa, weston seems to work. I'm not familiar with mesa and I don't know what I'm doing is actually right. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] clover: Add a space at the end of CL_DEVICE_OPENCL_C_VERSION
On Sat, 2015-03-21 at 00:35 +, Tom Stellard wrote: This is required by the spec. --- src/gallium/state_trackers/clover/api/device.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp index 5d1f4ab..04f293d 100644 --- a/src/gallium/state_trackers/clover/api/device.cpp +++ b/src/gallium/state_trackers/clover/api/device.cpp @@ -341,7 +341,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param, break; case CL_DEVICE_OPENCL_C_VERSION: - buf.as_string() = OpenCL C 1.1; + buf.as_string() = OpenCL C 1.1 ; break; case CL_DEVICE_PARENT_DEVICE: I guess since we don't provide any vendor specific info, this is correct. Reviewed-by: Jan Vesely jan.ves...@rutgers.edu -- Jan Vesely jan.ves...@rutgers.edu signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: Return 0 as storage size for local kernel args that are not set
Jan Vesely jan.ves...@rutgers.edu writes: On Sat, 2015-03-21 at 14:57 +0200, Francisco Jerez wrote: Jan Vesely jan.ves...@rutgers.edu writes: On Fri, 2015-03-20 at 23:29 +, Tom Stellard wrote: The storage size for local kernel args can be queried before the arguments are set by using the CL_KERNEL_LOCAL_MEM_SIZE param of clGetKernelWorkGroupInfo(). The spec says that if local kernel arguments have not been specified, then we should assume their size is 0. --- src/gallium/state_trackers/clover/core/kernel.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp b/src/gallium/state_trackers/clover/core/kernel.cpp index 442762c..f4f8616 100644 --- a/src/gallium/state_trackers/clover/core/kernel.cpp +++ b/src/gallium/state_trackers/clover/core/kernel.cpp @@ -399,6 +399,8 @@ kernel::global_argument::unbind(exec_context ctx) { size_t kernel::local_argument::storage() const { + if (!_set) + return 0; Shouldn't _storage be 0 before anybody calls local_argument::set anyway? maybe initializing it in a constructor would suffice? Yes, the local_argument class could really use a constructor. Not sure what the rules on using C++11 features are, but we could also use member initialization. in kernel.hpp: - size_t _storage; + size_t _storage = 0; Yes, that would be a fine solution, probably less typing than defining the constructor explicitly. jan jan return _storage; } -- Jan Vesely jan.ves...@rutgers.edu ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev -- Jan Vesely jan.ves...@rutgers.edu signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gbm: remove gbm_gallium_drm from the loader
Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- Should have sent with the remove st/gbm series. Ohh well. -Emil src/gbm/main/backend.c | 24 1 file changed, 24 deletions(-) diff --git a/src/gbm/main/backend.c b/src/gbm/main/backend.c index aceb662..4929d73 100644 --- a/src/gbm/main/backend.c +++ b/src/gbm/main/backend.c @@ -30,7 +30,6 @@ #include stdlib.h #include string.h #include limits.h -#include dlfcn.h #include backend.h @@ -45,41 +44,18 @@ struct backend_desc { static const struct backend_desc backends[] = { { gbm_dri.so, gbm_dri_backend }, - { gbm_gallium_drm.so, NULL }, }; static const void * load_backend(const struct backend_desc *backend) { - char path[PATH_MAX]; const void *init = NULL; - void *module; - const char *name; - const char *entrypoint = gbm_backend; if (backend == NULL) return NULL; - name = backend-name; - if (backend-builtin) { init = backend-builtin; - } else { - if (name[0] != '/') - snprintf(path, sizeof path, MODULEDIR /%s, name); - else - snprintf(path, sizeof path, %s, name); - - module = dlopen(path, RTLD_NOW | RTLD_GLOBAL); - if (!module) { - fprintf(stderr, - failed to load module: %s\n, dlerror()); - return NULL; - } - - init = dlsym(module, entrypoint); - if (!init) - return NULL; } return init; -- 2.1.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa/state_tracker: Fix draw-pixel-with-texture piglit test.
When glDrawPixels was used with an external texture, the pixels passed in were sampled instead of the texture. Change gallium to instead move the user texture to a new sampler below the glDrawPixels samplers. While the piglit test now works, the texture coordinates that are used to sample the texture are not the ones provided by the user, and need to be updated as well. Tested on llvmpipe, r600, and radeonsi. --- src/mesa/state_tracker/st_cb_drawpixels.c | 11 ++- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 14 ++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/mesa/state_tracker/st_cb_drawpixels.c b/src/mesa/state_tracker/st_cb_drawpixels.c index 3edf31b..09d4b9d 100644 --- a/src/mesa/state_tracker/st_cb_drawpixels.c +++ b/src/mesa/state_tracker/st_cb_drawpixels.c @@ -676,6 +676,7 @@ draw_textured_quad(struct gl_context *ctx, GLint x, GLint y, GLfloat z, GLfloat x0, y0, x1, y1; GLsizei maxSize; boolean normalized = sv[0]-texture-target != PIPE_TEXTURE_RECT; + int num_user_sampers = st-state.num_samplers[PIPE_SHADER_FRAGMENT]; /* limit checks */ /* XXX if DrawPixels image is larger than max texture size, break @@ -765,6 +766,9 @@ draw_textured_quad(struct gl_context *ctx, GLint x, GLint y, GLfloat z, if (num_sampler_view 1) { cso_single_sampler(cso, PIPE_SHADER_FRAGMENT, 1, sampler); } + for(int i = 0; i num_user_sampers; ++i) { + cso_single_sampler(cso, PIPE_SHADER_FRAGMENT, i+num_sampler_view, st-state.samplers[PIPE_SHADER_FRAGMENT][i]); + } cso_single_sampler_done(cso, PIPE_SHADER_FRAGMENT); } @@ -786,7 +790,12 @@ draw_textured_quad(struct gl_context *ctx, GLint x, GLint y, GLfloat z, cso_set_stream_outputs(st-cso_context, 0, NULL, NULL); /* texture state: */ - cso_set_sampler_views(cso, PIPE_SHADER_FRAGMENT, num_sampler_view, sv); + { + struct pipe_sampler_view *lsv[PIPE_MAX_SAMPLERS]; + memcpy(lsv, sv, num_sampler_view*sizeof(struct pipe_sampler_view*)); + memcpy(lsv+num_sampler_view, st- state.sampler_views[PIPE_SHADER_FRAGMENT], num_user_sampers*sizeof(struct pipe_sampler_view*)); + cso_set_sampler_views(cso, PIPE_SHADER_FRAGMENT, num_sampler_view+num_user_sampers, lsv); + } /* Compute Gallium window coords (y=0=top) with pixel zoom. * Recall that these coords are transformed by the current diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index efee4b2..bbb13ae 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -4254,6 +4254,7 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp, st_src_reg coord, src0; st_dst_reg dst0; glsl_to_tgsi_instruction *inst; + unsigned int count_samplers_used = 0; /* Copy attributes of the glsl_to_tgsi_visitor in the original shader. */ v-ctx = original-ctx; @@ -4284,6 +4285,7 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp, prog-InputsRead |= VARYING_BIT_TEX0; prog-SamplersUsed |= (1 0); /* mark sampler 0 as used */ v-samplers_used |= (1 0); + count_samplers_used++; if (scale_and_bias) { static const gl_state_index scale_state[STATE_LENGTH] = @@ -4331,6 +4333,7 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp, prog-SamplersUsed |= (1 1); /* mark sampler 1 as used */ v-samplers_used |= (1 1); + count_samplers_used++; /* MOV colorTemp, temp; */ inst = v-emit(NULL, TGSI_OPCODE_MOV, dst0, temp); @@ -4359,6 +4362,17 @@ get_pixel_transfer_visitor(struct st_fragment_program *fp, newinst = v-emit(NULL, inst-op, inst-dst[0], src_regs[0], src_regs[1], src_regs[2]); newinst-tex_target = inst-tex_target; newinst-sampler_array_size = inst-sampler_array_size; + + if (newinst-tex_target != 0) { + int new_sampler_index = newinst-sampler.index + count_samplers_used; + newinst-sampler.index = new_sampler_index; + + if ((prog-SamplersUsed (1 new_sampler_index)) == 0) { +_mesa_add_parameter(params, PROGRAM_SAMPLER, sampler_1, 4, 35678, 0, 0); + } + prog-SamplersUsed |= (1 new_sampler_index); + v-samplers_used |= (1 new_sampler_index); + } } /* Make modifications to fragment program info. */ -- 2.0.5 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] More ARB_arrays_of_arrays support
On Mar 21, 2015 2:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: This series adds most of the remaining glsl arrays of arrays support. Support for uniform blocks is still missing, I've played around with this but don't have anything working yet. What do you guys think about adding these changes without the uniform blocks support? Adding these changes now I assume would make it easy for the nir guys to start adding AoA support (if it doesn't exist already) and the intel backend changes could start to be worked on. Support in NIR should just work. Of course that probably won't actually be the case. However, if it does break something then that's a bug because the code is all there. It should be easy to check since your already running on Intel. If your i965 is IVB+, just do a piglit run with INTEL_USE_NIR=1. If its older than that, I'll be pushing support for SNB and prior on Monday; just rebase. The one place I know of where this will get dicey is in the sampler arrays portion of the ARB_gpu_shader5 extension. That implementation is rather hacky and I'm pretty sure will fall flat on its face if milti-level arrays are involved. I know NIR's implementation will assert-fail; the i965 code may just do the wrong thing. In other words, tests are needed for that specific case. However, I guess that falls under the category of yet-to-be-done uniform stuff. For anyone interested the main remaining changes to be made are in: ./glsl/link_uniform_block_active_visitor.cpp ./glsl/link_uniform_blocks.cpp ./glsl/lower_ubo_reference.cpp For gallium drivers the glsl to tgsi conversion seems to handle everything without changes so at least for the piglit tests I've written everything seem to just work. There are no piglit regressions from these changes (tested on i965 driver) and all the current arrays of arrays piglit test pass (tested with ilo gallium driver using MESA_GL_VERSION_OVERRIDE=3.3COMPAT MESA_GLSL_VERSION_OVERRIDE=150) besides the uniform block tests. There are also some unreviewed interface linking tests relating to the last 4 patches on the piglit list [1]. I also have some generated execution tests in my piglit repo [2], I'm not sure if I've created these the best way and wish to extend them further so these haven't been sent to the piglit list yet. [1] http://lists.freedesktop.org/archives/piglit/2015-March/015167.html [2] https://github.com/tarceri/Mesa_arrays_of_arrays_piglit/tree/execution_tests ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/util: Define ffsll on OpenBSD.
On 17/03/15 23:44, Emil Velikov wrote: On 17/03/15 01:25, Jonathan Gray wrote: On Mon, Mar 16, 2015 at 08:37:28PM +, Emil Velikov wrote: On 26/02/15 13:49, Jose Fonseca wrote: On 26/02/15 13:42, Jose Fonseca wrote: On 26/02/15 03:55, Jonathan Gray wrote: On Wed, Feb 25, 2015 at 07:09:26PM -0800, Matt Turner wrote: On Wed, Feb 25, 2015 at 7:03 PM, Jonathan Gray j...@jsg.id.au wrote: On Wed, Feb 25, 2015 at 06:53:14PM -0800, Matt Turner wrote: On Wed, Feb 25, 2015 at 5:37 PM, Jonathan Gray j...@jsg.id.au wrote: If it isn't going to be configure checks could someone merge the original patch in this thread? I committed commit 3492e88090d2d0c0bfbc934963b8772b45fc8880 Author: Matt Turner matts...@gmail.com Date: Fri Feb 20 18:46:43 2015 -0800 gallium/util: Use HAVE___BUILTIN_* macros. Reviewed-by: Eric Anholt e...@anholt.net Reviewed-by: Jose Fonseca jfons...@vmware.com which switched over a bunch of preprocessor checks around __builtin* calls to use the macros defined by autotools. So I think cleaning it up to use __builtin_ffs* first #ifdef HAVE___BUILTIN_* can go forward now. Yes but there is no HAVE_FFSLL for constructs like #if !defined(HAVE_FFSLL) defined(HAVE___BUILTIN_FFSLL) or is it ok to always use the builtin? I think the question is whether it's okay to always use the builtin if it's available (as opposed to libc functions). I think the answer to that is yes. So in that case how about the following? Or is it going to break the android scons build? From cba39ba72115e57d262cb4b099c4e72106f01812 Mon Sep 17 00:00:00 2001 From: Jonathan Gray j...@jsg.id.au Date: Thu, 26 Feb 2015 14:46:45 +1100 Subject: [PATCH] gallium/util: use ffs* builtins if available Required to build on OpenBSD which doesn't have ffsll in libc. Signed-off-by: Jonathan Gray j...@jsg.id.au --- src/gallium/auxiliary/util/u_math.h | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h index b4a65e4..5bc9b97 100644 --- a/src/gallium/auxiliary/util/u_math.h +++ b/src/gallium/auxiliary/util/u_math.h @@ -384,9 +384,6 @@ unsigned ffs( unsigned u ) return i; } -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) -#define ffs __builtin_ffs -#define ffsll __builtin_ffsll Scons does define HAVE___BUILTIN_FFS for mingw. However `git grep '\ffs\` shows ffs is used directly in many other places. So I suspect this change will break them. #endif #endif /* FFS_DEFINED */ @@ -435,7 +432,11 @@ util_last_bit_signed(int i) static INLINE int u_bit_scan(unsigned *mask) { +#if defined(HAVE___BUILTIN_FFS) + int i = __builtin_ffs(*mask) - 1; +#else int i = ffs(*mask) - 1; +#endif *mask = ~(1 i); return i; } @@ -444,7 +445,11 @@ u_bit_scan(unsigned *mask) static INLINE int u_bit_scan64(uint64_t *mask) { +#if defined(HAVE___BUILTIN_FFSLL) + int i = __builtin_ffsll(*mask) - 1; +#else int i = ffsll(*mask) - 1; +#endif *mask = ~(1llu i); return i; } I think the right thing long term is to provide ffs and ffsll in c99_compat.h or c99_math.h for all platforms. And let the rest of the code just always assume it's available somehow. Otherwise, let's just '#define ffs __builtin_ffs' on OpenBSD too. In other words, the original patch on this thread http://lists.freedesktop.org/archives/mesa-dev/2015-February/076071.html is the only patch I've seen so far that doesn't break Mingw. If you rather use HAVE___BUILTIN_FFSLL, then just do diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h index 959f76e..d372cfd 100644 --- a/src/gallium/auxiliary/util/u_math.h +++ b/src/gallium/auxiliary/util/u_math.h @@ -384,7 +384,7 @@ unsigned ffs( unsigned u ) return i; } -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) +#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) || defined(HAVE___BUILTIN_FFSLL) #define ffs __builtin_ffs #define ffsll __builtin_ffsll #endif Jonathan Seems like this has ended up a longer discussion that anticipated :\ Can you please confirm if the above works for you ? Thanks Emil It looks like that diff was mangled by the mail client and doesn't have the newline escaped. It also assumes a ffsll builtin implies a ffs builtin is present. So how about the following instead: diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h index 8f62cac..89c63d7 100644 --- a/src/gallium/auxiliary/util/u_math.h +++ b/src/gallium/auxiliary/util/u_math.h @@ -383,14 +383,28 @@ unsigned ffs( unsigned u ) return i; } -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) +#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) || \ +defined(HAVE___BUILTIN_FFS) #define ffs __builtin_ffs -#define ffsll __builtin_ffsll #endif #endif /*
Re: [Mesa-dev] [PATCH 3/3] radeonsi/compute: Default to the same PIPE_SHADER_CAP values as other shader types
Le samedi 21 mars 2015, 00:35:08 Tom Stellard a écrit : --- src/gallium/drivers/radeonsi/si_pipe.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/gallium/drivers/radeonsi/si_pipe.c b/src/gallium/drivers/radeonsi/si_pipe.c index f1a5388..545c156 100644 --- a/src/gallium/drivers/radeonsi/si_pipe.c +++ b/src/gallium/drivers/radeonsi/si_pipe.c @@ -373,8 +373,12 @@ static int si_get_shader_param(struct pipe_screen* pscreen, unsigned shader, enu return max_const_buffer_size; } default: - return 0; + /* If compute shaders don't require a speical value There is a typo here: speical + * for this cap, we can return the same value we + * do for other shader types. */ + break; } + break; default: /* TODO: support tessellation */ return 0; -- Laurent Carlier http://www.archlinux.org signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/4] i965: Rename do_stage_prog to brw_stage_compile
On Fri, Mar 20 2015, Chris Forbes wrote: I think that having both the existing `struct brw_vs_compile` and a function with the same name is going to cause confusion. (same with the other non-fs stages) In an earlier version of the patch I had brw_vs_do_compile, (there is a do precedent in the code being replaced here). I could go back to that if it helps. -Carl pgpmHnTz9onNG.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/4] i965: Split out brw_stage_populate_key into their own functions
On Fri, Mar 20 2015, Ian Romanick wrote: +brw_gs_populate_key(struct brw_context *brw, +struct brw_gs_prog_key *key) Tabs. There may be some in other places too. Thunderbird's editor isn't too smart to be able to search for tabs... it matches any whitespace. Sorry about that! I don't know what changed, but recently the .dir-locals.el file stopped having any effect on emacs for me like it should, (and like it used to). Does any other emacs user have an idea for me? So I've been manually changing the indentation from 8 to 3, but I hadn't realized I also needed to be changing the setting to use spaces instead of tabs. I'll fix that for the series, (and hopefully fix this to get set automatically once again). -Carl pgpxwIrzDk1ZL.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] gallium/util: Define ffsll on OpenBSD.
On Sat, Mar 21, 2015 at 10:10 AM, Emil Velikov emil.l.veli...@gmail.com wrote: On 17/03/15 23:44, Emil Velikov wrote: On 17/03/15 01:25, Jonathan Gray wrote: On Mon, Mar 16, 2015 at 08:37:28PM +, Emil Velikov wrote: On 26/02/15 13:49, Jose Fonseca wrote: On 26/02/15 13:42, Jose Fonseca wrote: On 26/02/15 03:55, Jonathan Gray wrote: On Wed, Feb 25, 2015 at 07:09:26PM -0800, Matt Turner wrote: On Wed, Feb 25, 2015 at 7:03 PM, Jonathan Gray j...@jsg.id.au wrote: On Wed, Feb 25, 2015 at 06:53:14PM -0800, Matt Turner wrote: On Wed, Feb 25, 2015 at 5:37 PM, Jonathan Gray j...@jsg.id.au wrote: If it isn't going to be configure checks could someone merge the original patch in this thread? I committed commit 3492e88090d2d0c0bfbc934963b8772b45fc8880 Author: Matt Turner matts...@gmail.com Date: Fri Feb 20 18:46:43 2015 -0800 gallium/util: Use HAVE___BUILTIN_* macros. Reviewed-by: Eric Anholt e...@anholt.net Reviewed-by: Jose Fonseca jfons...@vmware.com which switched over a bunch of preprocessor checks around __builtin* calls to use the macros defined by autotools. So I think cleaning it up to use __builtin_ffs* first #ifdef HAVE___BUILTIN_* can go forward now. Yes but there is no HAVE_FFSLL for constructs like #if !defined(HAVE_FFSLL) defined(HAVE___BUILTIN_FFSLL) or is it ok to always use the builtin? I think the question is whether it's okay to always use the builtin if it's available (as opposed to libc functions). I think the answer to that is yes. So in that case how about the following? Or is it going to break the android scons build? From cba39ba72115e57d262cb4b099c4e72106f01812 Mon Sep 17 00:00:00 2001 From: Jonathan Gray j...@jsg.id.au Date: Thu, 26 Feb 2015 14:46:45 +1100 Subject: [PATCH] gallium/util: use ffs* builtins if available Required to build on OpenBSD which doesn't have ffsll in libc. Signed-off-by: Jonathan Gray j...@jsg.id.au --- src/gallium/auxiliary/util/u_math.h | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h index b4a65e4..5bc9b97 100644 --- a/src/gallium/auxiliary/util/u_math.h +++ b/src/gallium/auxiliary/util/u_math.h @@ -384,9 +384,6 @@ unsigned ffs( unsigned u ) return i; } -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) -#define ffs __builtin_ffs -#define ffsll __builtin_ffsll Scons does define HAVE___BUILTIN_FFS for mingw. However `git grep '\ffs\` shows ffs is used directly in many other places. So I suspect this change will break them. #endif #endif /* FFS_DEFINED */ @@ -435,7 +432,11 @@ util_last_bit_signed(int i) static INLINE int u_bit_scan(unsigned *mask) { +#if defined(HAVE___BUILTIN_FFS) + int i = __builtin_ffs(*mask) - 1; +#else int i = ffs(*mask) - 1; +#endif *mask = ~(1 i); return i; } @@ -444,7 +445,11 @@ u_bit_scan(unsigned *mask) static INLINE int u_bit_scan64(uint64_t *mask) { +#if defined(HAVE___BUILTIN_FFSLL) + int i = __builtin_ffsll(*mask) - 1; +#else int i = ffsll(*mask) - 1; +#endif *mask = ~(1llu i); return i; } I think the right thing long term is to provide ffs and ffsll in c99_compat.h or c99_math.h for all platforms. And let the rest of the code just always assume it's available somehow. Otherwise, let's just '#define ffs __builtin_ffs' on OpenBSD too. In other words, the original patch on this thread http://lists.freedesktop.org/archives/mesa-dev/2015-February/076071.html is the only patch I've seen so far that doesn't break Mingw. If you rather use HAVE___BUILTIN_FFSLL, then just do diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h index 959f76e..d372cfd 100644 --- a/src/gallium/auxiliary/util/u_math.h +++ b/src/gallium/auxiliary/util/u_math.h @@ -384,7 +384,7 @@ unsigned ffs( unsigned u ) return i; } -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) +#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) || defined(HAVE___BUILTIN_FFSLL) #define ffs __builtin_ffs #define ffsll __builtin_ffsll #endif Jonathan Seems like this has ended up a longer discussion that anticipated :\ Can you please confirm if the above works for you ? Thanks Emil It looks like that diff was mangled by the mail client and doesn't have the newline escaped. It also assumes a ffsll builtin implies a ffs builtin is present. So how about the following instead: diff --git a/src/gallium/auxiliary/util/u_math.h b/src/gallium/auxiliary/util/u_math.h index 8f62cac..89c63d7 100644 --- a/src/gallium/auxiliary/util/u_math.h +++ b/src/gallium/auxiliary/util/u_math.h @@ -383,14 +383,28 @@ unsigned ffs( unsigned u ) return i; } -#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) +#elif defined(__MINGW32__) || defined(PIPE_OS_ANDROID) || \ +defined(HAVE___BUILTIN_FFS)
Re: [Mesa-dev] [RFC PATCH 0/7] Prune stale components, take 2
On 14/03/15 22:36, Emil Velikov wrote: Hi all, As a follow up to Jose's initial series dropping st/{egl,vega} and others here is the next round of cleanups, Namely this series - removes st/gbm, winsys/sw/{wayland,fbdev}, all of which no longer used by anyone, - cleans up libEGL (and configure) of the final fbdev references, - cleans up the documentation and - removes the galahad driver. All gone now. Jose, Matt, thanks for having a look. I've pushed the series plus a small patch on top to update the release notes. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: Return 0 as storage size for local kernel args that are not set
On Sat, 2015-03-21 at 14:57 +0200, Francisco Jerez wrote: Jan Vesely jan.ves...@rutgers.edu writes: On Fri, 2015-03-20 at 23:29 +, Tom Stellard wrote: The storage size for local kernel args can be queried before the arguments are set by using the CL_KERNEL_LOCAL_MEM_SIZE param of clGetKernelWorkGroupInfo(). The spec says that if local kernel arguments have not been specified, then we should assume their size is 0. --- src/gallium/state_trackers/clover/core/kernel.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp b/src/gallium/state_trackers/clover/core/kernel.cpp index 442762c..f4f8616 100644 --- a/src/gallium/state_trackers/clover/core/kernel.cpp +++ b/src/gallium/state_trackers/clover/core/kernel.cpp @@ -399,6 +399,8 @@ kernel::global_argument::unbind(exec_context ctx) { size_t kernel::local_argument::storage() const { + if (!_set) + return 0; Shouldn't _storage be 0 before anybody calls local_argument::set anyway? maybe initializing it in a constructor would suffice? Yes, the local_argument class could really use a constructor. Not sure what the rules on using C++11 features are, but we could also use member initialization. in kernel.hpp: - size_t _storage; + size_t _storage = 0; jan jan return _storage; } -- Jan Vesely jan.ves...@rutgers.edu ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev -- Jan Vesely jan.ves...@rutgers.edu signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: Return 0 as storage size for local kernel args that are not set
Jan Vesely jan.ves...@rutgers.edu writes: On Fri, 2015-03-20 at 23:29 +, Tom Stellard wrote: The storage size for local kernel args can be queried before the arguments are set by using the CL_KERNEL_LOCAL_MEM_SIZE param of clGetKernelWorkGroupInfo(). The spec says that if local kernel arguments have not been specified, then we should assume their size is 0. --- src/gallium/state_trackers/clover/core/kernel.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp b/src/gallium/state_trackers/clover/core/kernel.cpp index 442762c..f4f8616 100644 --- a/src/gallium/state_trackers/clover/core/kernel.cpp +++ b/src/gallium/state_trackers/clover/core/kernel.cpp @@ -399,6 +399,8 @@ kernel::global_argument::unbind(exec_context ctx) { size_t kernel::local_argument::storage() const { + if (!_set) + return 0; Shouldn't _storage be 0 before anybody calls local_argument::set anyway? maybe initializing it in a constructor would suffice? Yes, the local_argument class could really use a constructor. jan return _storage; } -- Jan Vesely jan.ves...@rutgers.edu ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 18/23] glsl: Constify ir_instruction::equals
Ian Romanick i...@freedesktop.org writes: From: Ian Romanick ian.d.roman...@intel.com The casts to remove const from ir are ugly. The alternative was to add const versions of all the as_foo functions, and I just couldn't get up the motivation to do that. Wouldn't the alternative be roughly as much typing as casting const away? You define these conversion methods using a macro so you could probably just fix it there to get const overloads for all of them and avoid the ugly casts. If there's a third option that is better, I would love to hear about it. :) Signed-off-by: Ian Romanick ian.d.roman...@intel.com Cc: Francisco Jerez curroje...@riseup.net --- src/glsl/ir.h | 21 ++--- src/glsl/ir_equals.cpp | 34 -- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/src/glsl/ir.h b/src/glsl/ir.h index fdc22ed..385d661 100644 --- a/src/glsl/ir.h +++ b/src/glsl/ir.h @@ -183,7 +183,8 @@ public: * in particular. No support for other instruction types (assignments, * jumps, calls, etc.) is planned. */ - virtual bool equals(ir_instruction *ir, enum ir_node_type ignore = ir_type_unset); + virtual bool equals(const ir_instruction *ir, + enum ir_node_type ignore = ir_type_unset) const; protected: ir_instruction(enum ir_node_type t) @@ -1598,7 +1599,8 @@ public: */ ir_expression(int op, ir_rvalue *op0, ir_rvalue *op1, ir_rvalue *op2); - virtual bool equals(ir_instruction *ir, enum ir_node_type ignore = ir_type_unset); + virtual bool equals(const ir_instruction *ir, + enum ir_node_type ignore = ir_type_unset) const; virtual ir_expression *clone(void *mem_ctx, struct hash_table *ht) const; @@ -1909,7 +1911,8 @@ public: virtual ir_visitor_status accept(ir_hierarchical_visitor *); - virtual bool equals(ir_instruction *ir, enum ir_node_type ignore = ir_type_unset); + virtual bool equals(const ir_instruction *ir, + enum ir_node_type ignore = ir_type_unset) const; /** * Return a string representing the ir_texture_opcode. @@ -2010,7 +2013,8 @@ public: virtual ir_visitor_status accept(ir_hierarchical_visitor *); - virtual bool equals(ir_instruction *ir, enum ir_node_type ignore = ir_type_unset); + virtual bool equals(const ir_instruction *ir, + enum ir_node_type ignore = ir_type_unset) const; bool is_lvalue() const { @@ -2063,7 +2067,8 @@ public: virtual ir_constant *constant_expression_value(struct hash_table *variable_context = NULL); - virtual bool equals(ir_instruction *ir, enum ir_node_type ignore = ir_type_unset); + virtual bool equals(const ir_instruction *ir, + enum ir_node_type ignore = ir_type_unset) const; /** * Get the variable that is ultimately referenced by an r-value @@ -2109,7 +2114,8 @@ public: virtual ir_constant *constant_expression_value(struct hash_table *variable_context = NULL); - virtual bool equals(ir_instruction *ir, enum ir_node_type ignore = ir_type_unset); + virtual bool equals(const ir_instruction *ir, + enum ir_node_type ignore = ir_type_unset) const; /** * Get the variable that is ultimately referenced by an r-value @@ -2219,7 +2225,8 @@ public: virtual ir_visitor_status accept(ir_hierarchical_visitor *); - virtual bool equals(ir_instruction *ir, enum ir_node_type ignore = ir_type_unset); + virtual bool equals(const ir_instruction *ir, + enum ir_node_type ignore = ir_type_unset) const; /** * Get a particular component of a constant as a specific type diff --git a/src/glsl/ir_equals.cpp b/src/glsl/ir_equals.cpp index 65376cd..d8066a0 100644 --- a/src/glsl/ir_equals.cpp +++ b/src/glsl/ir_equals.cpp @@ -28,7 +28,8 @@ * can't access a's vtable in that case. */ static bool -possibly_null_equals(ir_instruction *a, ir_instruction *b, enum ir_node_type ignore) +possibly_null_equals(const ir_instruction *a, const ir_instruction *b, + enum ir_node_type ignore) { if (!a || !b) return !a !b; @@ -41,15 +42,15 @@ possibly_null_equals(ir_instruction *a, ir_instruction *b, enum ir_node_type ign * about. */ bool -ir_instruction::equals(ir_instruction *, enum ir_node_type) +ir_instruction::equals(const ir_instruction *, enum ir_node_type) const { return false; } bool -ir_constant::equals(ir_instruction *ir, enum ir_node_type) +ir_constant::equals(const ir_instruction *ir, enum ir_node_type) const { - const ir_constant *other = ir-as_constant(); + const ir_constant *other = ((ir_instruction *)ir)-as_constant(); if (!other) return false; @@ -65,9 +66,11 @@ ir_constant::equals(ir_instruction *ir, enum ir_node_type) }
Re: [Mesa-dev] [PATCH] gallium/targets/d3dadapter9: drop the libdrm prefix for drm.h
Looks good to me. :) On 03/20/2015 10:20 PM, Emil Velikov wrote: The path is provided by libdrm.pc and already used appropriately by the build system. Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- src/gallium/targets/d3dadapter9/drm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/targets/d3dadapter9/drm.c b/src/gallium/targets/d3dadapter9/drm.c index ec594fd..6342ab8 100644 --- a/src/gallium/targets/d3dadapter9/drm.c +++ b/src/gallium/targets/d3dadapter9/drm.c @@ -39,7 +39,7 @@ #include xmlconfig.h #include xmlpool.h -#include libdrm/drm.h +#include drm.h #include sys/ioctl.h #include fcntl.h #include stdio.h ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89699] Regression in 10.5.1 causes flickering/artifacting in a particular video game
https://bugs.freedesktop.org/show_bug.cgi?id=89699 Michel Dänzer mic...@daenzer.net changed: What|Removed |Added Component|Other |Drivers/Gallium/radeonsi Assignee|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop |org |.org QA Contact|mesa-dev@lists.freedesktop. |dri-devel@lists.freedesktop |org |.org --- Comment #3 from Michel Dänzer mic...@daenzer.net --- Does it still happen with current Mesa Git master? If so, can you bisect? -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] clover: Add a space at the end of CL_DEVICE_OPENCL_C_VERSION
Tom Stellard thomas.stell...@amd.com writes: This is required by the spec. Déjà-vu? Reviewed-by: Francisco Jerez curroje...@riseup.net --- src/gallium/state_trackers/clover/api/device.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp index 5d1f4ab..04f293d 100644 --- a/src/gallium/state_trackers/clover/api/device.cpp +++ b/src/gallium/state_trackers/clover/api/device.cpp @@ -341,7 +341,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param, break; case CL_DEVICE_OPENCL_C_VERSION: - buf.as_string() = OpenCL C 1.1; + buf.as_string() = OpenCL C 1.1 ; break; case CL_DEVICE_PARENT_DEVICE: -- 2.0.4 signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] clover: Add all the mandatory 1.1 extensions to the extension string
Tom Stellard thomas.stell...@amd.com writes: --- src/gallium/state_trackers/clover/api/device.cpp | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp index 04f293d..43e7475 100644 --- a/src/gallium/state_trackers/clover/api/device.cpp +++ b/src/gallium/state_trackers/clover/api/device.cpp @@ -301,7 +301,13 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param, break; case CL_DEVICE_EXTENSIONS: - buf.as_string() = dev.has_doubles() ? cl_khr_fp64 : ; + buf.as_string() = + cl_khr_global_int32_base_atomics + cl_khr_global_int32_extended_atomics + cl_khr_local_int32_base_atomics + cl_khr_local_int32_extended_atomics + cl_khr_byte_addressable_store + + std::string(dev.has_doubles() ? cl_khr_fp64 : ); break; Reviewed-by: Francisco Jerez curroje...@riseup.net case CL_DEVICE_PLATFORM: -- 2.0.4 signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: The unit for CL_DEVICE_MEM_BASE_ADDR_ALIGN is bits not bytes
Tom Stellard thomas.stell...@amd.com writes: --- src/gallium/state_trackers/clover/api/device.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp index 43e7475..bc93f91 100644 --- a/src/gallium/state_trackers/clover/api/device.cpp +++ b/src/gallium/state_trackers/clover/api/device.cpp @@ -196,6 +196,9 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param, break; case CL_DEVICE_MEM_BASE_ADDR_ALIGN: + buf.as_scalarcl_uint() = 128 * 8; + break; + case CL_DEVICE_MIN_DATA_TYPE_ALIGN_SIZE: buf.as_scalarcl_uint() = 128; break; -- 2.0.4 Reviewed-by: Francisco Jerez curroje...@riseup.net signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 7/7] i965/nir: Use signed integer type for booleans
On Fri, Mar 20, 2015 at 2:21 PM, Jason Ekstrand ja...@jlekstrand.net wrote: On Fri, Mar 20, 2015 at 1:56 PM, Matt Turner matts...@gmail.com wrote: On Fri, Mar 20, 2015 at 1:37 PM, Jason Ekstrand ja...@jlekstrand.net wrote: On Fri, Mar 20, 2015 at 1:12 PM, Matt Turner matts...@gmail.com wrote: On Fri, Mar 20, 2015 at 11:24 AM, Jason Ekstrand ja...@jlekstrand.net wrote: - emit(MOV(result, masked)); + emit(MOV(retype(result, BRW_REGISTER_TYPE_D), masked)); This hunk isn't necessary. masked is vgrf(glsl_type::int_type). This hunk retypes result, not masked. Oh, yeah. But result is set at the top of the function like this: fs_reg result = get_nir_dest(instr-dest.dest); result.type = brw_type_for_nir_type(nir_op_infos[instr-op].output_type); Isn't that sufficient? We're only going to be resolving things that were bool-typed to begin with, which should mean that the other changes in this patch handled it. I just looked at it and remembered what the problem was. Right now, iand, ior, and ixor and defined to take and produce unsigned types. Why? I don't know. I guess they seemed more like unsigned operations to Connor. We could change that easily enough to make them signed, but leaving this hunk in makes the FS backend do the right thing regardless. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/23] Handle Booleans and things better
On Friday, March 20, 2015 04:35:11 PM Jason Ekstrand wrote: On Fri, Mar 20, 2015 at 4:26 PM, Ian Romanick i...@freedesktop.org wrote: On 03/20/2015 03:10 PM, Jason Ekstrand wrote: There are a couple of reasons I'm asking for this. One is that NIR may be able to CSE something that results in one of these patterns and we wouldn't get the optimization. Another reason is that with Ken's work to run ARB programs through NIR, we would like those to get as many optimizations as the can and they b2f the universe. As an aside, That doesn't seem right. There's no bool in ARB programs, so how are they getting b2f? Exactly... which is why Ken's translator b2f's the universe. You'd have to double-check with him, but I'm pretty sure that the ARB version of (a b), which returns a float, is implemented as b2f(a b). At the very least, it's lowered to that so that the backend doesn't have to deal with float comparisons. Then NIR cleans it up quite a bit before our backends see it. Ken already killed 10% by going through NIR. It'd be nice to kill a few more. :-) On platforms that have native integer support, I made prog_to_nir use b2f(f__(x)) for SLT, SGE, SLE, SGT, SEQ, SNE. Also, CMP uses bcsel(flt(x), y, z)...and KIL uses bany4 instead of fany4. The thinking is to separate out the actual operation from the bools as floats part, so hopefully some of that can be optimized away. It also saves backends from having to implement SLT and friends. I've been hoping to go through and add algebraic optimizations for common ARB patterns (i.e. fmul(b2f(...)) - b2f(iand(...))). --Ken signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/15] i965/vec4: Simplify reswizzle() using the swizzle utils.
On Fri, Mar 20, 2015 at 7:11 AM, Francisco Jerez curroje...@riseup.net wrote: --- src/mesa/drivers/dri/i965/brw_vec4.cpp | 40 ++ 1 file changed, 11 insertions(+), 29 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp index 60f85f8..3e5f6c4 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp @@ -998,43 +998,25 @@ vec4_instruction::can_reswizzle(int dst_writemask, void vec4_instruction::reswizzle(int dst_writemask, int swizzle) { - int new_writemask = 0; - int new_swizzle[4] = { 0 }; - - /* Dot product instructions write a single result into all channels. */ + /* Destination write mask doesn't correspond to source swizzle for the dot +* product and pack_bytes instructions. +*/ if (opcode != BRW_OPCODE_DP4 opcode != BRW_OPCODE_DPH - opcode != BRW_OPCODE_DP3 opcode != BRW_OPCODE_DP2) { + opcode != BRW_OPCODE_DP3 opcode != BRW_OPCODE_DP2 + opcode != VEC4_OPCODE_PACK_BYTES) { for (int i = 0; i 3; i++) { if (src[i].file == BAD_FILE || src[i].file == IMM) continue; - /* Destination write mask doesn't correspond to source swizzle for the - * pack_bytes instruction. - */ - if (opcode == VEC4_OPCODE_PACK_BYTES) -continue; - - for (int c = 0; c 4; c++) { -new_swizzle[c] = BRW_GET_SWZ(src[i].swizzle, BRW_GET_SWZ(swizzle, c)); - } - - src[i].swizzle = BRW_SWIZZLE4(new_swizzle[0], new_swizzle[1], - new_swizzle[2], new_swizzle[3]); + src[i].swizzle = brw_compose_swizzle(swizzle, src[i].swizzle); } } - for (int c = 0; c 4; c++) { - int bit = 1 BRW_GET_SWZ(swizzle, c); - /* Skip components of the swizzle not used by the dst. */ - if (!(dst_writemask (1 c))) - continue; - /* If we were populating this component, then populate the - * corresponding channel of the new dst. - */ - if (dst.writemask bit) - new_writemask |= (1 c); - } - dst.writemask = new_writemask; + /* Apply the specified swizzle and writemask to the original mask of +* written components. +*/ + dst.writemask = dst_writemask + brw_apply_swizzle_to_mask(swizzle, dst.writemask); I'd indent this to line up vertically with dst_writemask. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/15] i965: Define some useful swizzle helper functions.
On Fri, Mar 20, 2015 at 7:11 AM, Francisco Jerez curroje...@riseup.net wrote: This defines helper functions implementing some common swizzle transformations that are usually open-coded in the compiler back-end, causing a lot of clutter. Some optimization passes will become almost trivial implemented in terms of these functions (e.g. vec4_visitor::opt_reduce_swizzle()). --- Really nice clean ups. The series is Reviewed-by: Matt Turner matts...@gmail.com ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 2/7] i965/nir: Use emit_lrp for emitting flrp
On Friday, March 20, 2015 02:36:15 PM Jason Ekstrand wrote: On Fri, Mar 20, 2015 at 2:28 PM, Ian Romanick i...@freedesktop.org wrote: On 03/20/2015 02:26 PM, Jason Ekstrand wrote: On Fri, Mar 20, 2015 at 2:24 PM, Ian Romanick i...@freedesktop.org wrote: On 03/20/2015 11:24 AM, Jason Ekstrand wrote: --- src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp index 5d88fe7..a059dbb 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp @@ -1243,8 +1243,7 @@ fs_visitor::nir_emit_alu(nir_alu_instr *instr) break; case nir_op_flrp: - /* TODO emulate for gen 6 */ - inst = emit(LRP(result, op[2], op[1], op[0])); + inst = emit_lrp(result, op[0], op[1], op[2]); inst-saturate = instr-dest.saturate; I wouldn't mind if you made this follow the pattern we use many other places: emit_lrp(result, op[0], op[1], op[2]) -saturate = instr-dest.saturate; unless inst is used later. I did that for *everything* in the initial NIR - FS pass and Ken made me take it out. Hmm... he might complain about some of the patches I just sent out, then. I thought this was the cool new way. So did I... But then I got to go through a file and take out 20 or 30 of them. It's definitely not the prevaling style; cool new way is fairly apt. I think Curro started doing this a while back. I think someone else preferred the 'inst' temporary, but I don't recall who (maybe Eric?). Honestly, at this point, it doesn't bother me. If you guys like that style, and prefer doing it that way, that's fine with me. signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89705] lp_bld_debug.cpp:280:54: error: no matching function for call to ‘llvm?=::TargetMachine::getSubtargetImpl()=?UTF-8?Q?’
https://bugs.freedesktop.org/show_bug.cgi?id=89705 Bug ID: 89705 Summary: lp_bld_debug.cpp:280:54: error: no matching function for call to ‘llvm::TargetMachine::getSubtargetImpl()’ Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: All Status: NEW Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: v...@freedesktop.org QA Contact: mesa-dev@lists.freedesktop.org mesa: bbaf22a998f89eb720a3995fa2216512deed5f76 (master 10.6.0-devel) Build error with llvm-3.7.0svn. Compiling src/gallium/auxiliary/gallivm/lp_bld_debug.cpp ... src/gallium/auxiliary/gallivm/lp_bld_debug.cpp: In function ‘size_t disassemble(const void*, llvm::raw_ostream)’: src/gallium/auxiliary/gallivm/lp_bld_debug.cpp:280:54: error: no matching function for call to ‘llvm::TargetMachine::getSubtargetImpl()’ const TargetInstrInfo *TII = TM-getSubtargetImpl()-getInstrInfo(); ^ -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH 05/12] glsl: avoid hitting assert for arrays of arrays
On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: Also add TODO comment about adding proper support Signed-off-by: Timothy Arceri t_arc...@yahoo.com.au --- src/glsl/ir_set_program_inouts.cpp | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/glsl/ir_set_program_inouts.cpp b/src/glsl/ir_set_program_inouts.cpp index e877a20..49cf85d 100644 --- a/src/glsl/ir_set_program_inouts.cpp +++ b/src/glsl/ir_set_program_inouts.cpp @@ -194,6 +194,12 @@ ir_set_program_inouts_visitor::try_mark_partial_variable(ir_variable *var, type = type-fields.array; } + /* TODO: implement proper arrays of arrays support +* for now let the caller mark whole variable as used. +*/ + if (type-is_array() type-fields.array-is_array()) + return false; + Won't this also hit the GS case even without array-of-arrays? The subject should reflect that this is about in/out variable liveness calculation. /* The code below only handles: * * - Indexing into matrices -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH 06/12] glsl: dont allow gl_PerVertex to be redeclared as an array of arrays
On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: Signed-off-by: Timothy Arceri t_arc...@yahoo.com.au --- src/glsl/ast_to_hir.cpp | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 36f3eb7..7516e60 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -5595,7 +5595,11 @@ ast_interface_block::hir(exec_list *instructions, _mesa_shader_stage_to_string(state-stage)); } if (this-instance_name == NULL || - strcmp(this-instance_name, gl_in) != 0 || this-array_specifier == NULL) { + strcmp(this-instance_name, gl_in) != 0 || this-array_specifier == NULL || + (this-array_specifier-is_unsized_array + !this-array_specifier-array_dimensions.is_empty()) || Don't know a lot about AST, but does it make sense to have -is_unsized_array array_dimensions? When would that be true? Shouldn't this just be !dim.empty() without the is_unsized_array check? + (this-array_specifier-array_dimensions.tail_pred-prev != NULL + !this-array_specifier-array_dimensions.tail_pred-prev-is_head_sentinel())) { Perhaps all this would be easier to read if above you wrote foo_type *array = this-array_specifier; Not sure. Perhaps there are some helpers to make this easier to grok? _mesa_glsl_error(loc, state, gl_PerVertex input must be redeclared as gl_in[]); -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC] More ARB_arrays_of_arrays support
Patches 1, 2, 3, 4, 8, and 11 are Reviewed-by: Ilia Mirkin imir...@alum.mit.edu The rest I sent comments on, although even if you address my feedback, I'm not sufficiently familiar to r-b at least some of them. If you do decide to play with UBO's, you can use idr's excellent random_ubo script to stress-test the various cases. Helped me a lot on getting doubles to actually work reliably. On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: This series adds most of the remaining glsl arrays of arrays support. Support for uniform blocks is still missing, I've played around with this but don't have anything working yet. What do you guys think about adding these changes without the uniform blocks support? Adding these changes now I assume would make it easy for the nir guys to start adding AoA support (if it doesn't exist already) and the intel backend changes could start to be worked on. For anyone interested the main remaining changes to be made are in: ./glsl/link_uniform_block_active_visitor.cpp ./glsl/link_uniform_blocks.cpp ./glsl/lower_ubo_reference.cpp For gallium drivers the glsl to tgsi conversion seems to handle everything without changes so at least for the piglit tests I've written everything seem to just work. There are no piglit regressions from these changes (tested on i965 driver) and all the current arrays of arrays piglit test pass (tested with ilo gallium driver using MESA_GL_VERSION_OVERRIDE=3.3COMPAT MESA_GLSL_VERSION_OVERRIDE=150) besides the uniform block tests. There are also some unreviewed interface linking tests relating to the last 4 patches on the piglit list [1]. I also have some generated execution tests in my piglit repo [2], I'm not sure if I've created these the best way and wish to extend them further so these haven't been sent to the piglit list yet. [1] http://lists.freedesktop.org/archives/piglit/2015-March/015167.html [2] https://github.com/tarceri/Mesa_arrays_of_arrays_piglit/tree/execution_tests ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 14/15] i965/vec4: Some more trivial swizzle clean-up.
On Fri, Mar 20, 2015 at 7:11 AM, Francisco Jerez curroje...@riseup.net wrote: diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp index a6842fc..8e4df8c 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp @@ -572,12 +572,8 @@ vec4_visitor::pack_uniform_registers() continue; inst-src[i].reg = new_loc[src]; - -int sx = BRW_GET_SWZ(inst-src[i].swizzle, 0) + new_chan[src]; -int sy = BRW_GET_SWZ(inst-src[i].swizzle, 1) + new_chan[src]; -int sz = BRW_GET_SWZ(inst-src[i].swizzle, 2) + new_chan[src]; -int sw = BRW_GET_SWZ(inst-src[i].swizzle, 3) + new_chan[src]; -inst-src[i].swizzle = BRW_SWIZZLE4(sx, sy, sz, sw); +inst-src[i].swizzle += BRW_SWIZZLE4(new_chan[src], new_chan[src], Tab here. + new_chan[src], new_chan[src]); } } } ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] mesa: add OES_EGL_image_external_essl3 support
Signed-off-by: Ilia Mirkin imir...@alum.mit.edu --- Not sure what kind of testing is needed here... thought I'd just send this out though, as the extension is simple enough. Wasn't sure whether mesa already handles the requirements re marking the texture as incomplete if the sampler has funny settings. src/glsl/builtin_functions.cpp | 17 + src/glsl/builtin_types.cpp | 3 ++- src/glsl/glsl_lexer.ll | 2 +- src/glsl/glsl_parser_extras.cpp | 1 + src/glsl/glsl_parser_extras.h | 2 ++ src/mesa/main/extensions.c | 1 + 6 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/glsl/builtin_functions.cpp b/src/glsl/builtin_functions.cpp index 84bbdc2..24fd4d6 100644 --- a/src/glsl/builtin_functions.cpp +++ b/src/glsl/builtin_functions.cpp @@ -147,6 +147,14 @@ texture_external(const _mesa_glsl_parse_state *state) return state-OES_EGL_image_external_enable; } +static bool +texture_external_es3(const _mesa_glsl_parse_state *state) +{ + return state-OES_EGL_image_external_essl3_enable + state-es_shader + state-is_version(0, 300); +} + /** True if texturing functions with explicit LOD are allowed. */ static bool lod_exists_in_stage(const _mesa_glsl_parse_state *state) @@ -1387,6 +1395,8 @@ builtin_builder::create_builtins() _texture(ir_tex, v130, glsl_type::float_type, glsl_type::sampler2DRectShadow_type, glsl_type::vec3_type), +_texture(ir_tex, texture_external_es3, glsl_type::vec4_type, glsl_type::samplerExternalOES_type, glsl_type::vec2_type), + _texture(ir_txb, v130_fs_only, glsl_type::vec4_type, glsl_type::sampler1D_type, glsl_type::float_type), _texture(ir_txb, v130_fs_only, glsl_type::ivec4_type, glsl_type::isampler1D_type, glsl_type::float_type), _texture(ir_txb, v130_fs_only, glsl_type::uvec4_type, glsl_type::usampler1D_type, glsl_type::float_type), @@ -1546,6 +1556,9 @@ builtin_builder::create_builtins() _texture(ir_tex, v130, glsl_type::float_type, glsl_type::sampler2DRectShadow_type, glsl_type::vec4_type, TEX_PROJECT), +_texture(ir_tex, texture_external_es3, glsl_type::vec4_type, glsl_type::samplerExternalOES_type, glsl_type::vec3_type, TEX_PROJECT), +_texture(ir_tex, texture_external_es3, glsl_type::vec4_type, glsl_type::samplerExternalOES_type, glsl_type::vec4_type, TEX_PROJECT), + _texture(ir_txb, v130, glsl_type::vec4_type, glsl_type::sampler1D_type, glsl_type::vec2_type, TEX_PROJECT), _texture(ir_txb, v130, glsl_type::ivec4_type, glsl_type::isampler1D_type, glsl_type::vec2_type, TEX_PROJECT), _texture(ir_txb, v130, glsl_type::uvec4_type, glsl_type::usampler1D_type, glsl_type::vec2_type, TEX_PROJECT), @@ -1604,8 +1617,12 @@ builtin_builder::create_builtins() _texelFetch(texture_multisample, glsl_type::vec4_type, glsl_type::sampler2DMSArray_type, glsl_type::ivec3_type), _texelFetch(texture_multisample, glsl_type::ivec4_type, glsl_type::isampler2DMSArray_type, glsl_type::ivec3_type), _texelFetch(texture_multisample, glsl_type::uvec4_type, glsl_type::usampler2DMSArray_type, glsl_type::ivec3_type), + +_texelFetch(texture_external_es3, glsl_type::vec4_type, glsl_type::samplerExternalOES_type, glsl_type::ivec2_type), + NULL); + add_function(texelFetchOffset, _texelFetch(v130, glsl_type::vec4_type, glsl_type::sampler1D_type, glsl_type::int_type, glsl_type::int_type), _texelFetch(v130, glsl_type::ivec4_type, glsl_type::isampler1D_type, glsl_type::int_type, glsl_type::int_type), diff --git a/src/glsl/builtin_types.cpp b/src/glsl/builtin_types.cpp index fef86df..c1a52b5 100644 --- a/src/glsl/builtin_types.cpp +++ b/src/glsl/builtin_types.cpp @@ -328,7 +328,8 @@ _mesa_glsl_initialize_types(struct _mesa_glsl_parse_state *state) add_type(symbols, glsl_type::sampler2DArrayShadow_type); } - if (state-OES_EGL_image_external_enable) { + if (state-OES_EGL_image_external_enable || + state-OES_EGL_image_external_essl3_enable) { add_type(symbols, glsl_type::samplerExternalOES_type); } diff --git a/src/glsl/glsl_lexer.ll b/src/glsl/glsl_lexer.ll index 8dc3d10..30867cd 100644 --- a/src/glsl/glsl_lexer.ll +++ b/src/glsl/glsl_lexer.ll @@ -339,7 +339,7 @@ usamplerCubeArray KEYWORD_WITH_ALT(400, 0, 400, 0, yyextra-ARB_texture_cube_map samplerCubeArrayShadow KEYWORD_WITH_ALT(400, 0, 400, 0, yyextra-ARB_texture_cube_map_array_enable, SAMPLERCUBEARRAYSHADOW); samplerExternalOES { - if (yyextra-OES_EGL_image_external_enable) + if (yyextra-OES_EGL_image_external_enable || yyextra-OES_EGL_image_external_essl3_enable) return SAMPLEREXTERNALOES; else
Re: [Mesa-dev] [RFC PATCH 07/12] glsl: Add support for linking uniform arrays of arrays
On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: --- src/glsl/link_uniform_initializers.cpp | 51 -- src/glsl/link_uniforms.cpp | 28 +++ 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp index 6907384..7817314 100644 --- a/src/glsl/link_uniform_initializers.cpp +++ b/src/glsl/link_uniform_initializers.cpp @@ -100,6 +100,38 @@ copy_constant_to_storage(union gl_constant_value *storage, } void +copy_constant_array_to_storage(struct gl_uniform_storage *const storage, + const ir_constant *val, + unsigned int *idx, + unsigned int *array_elements, + unsigned int boolean_true) +{ + if (val-type-fields.array-is_array()) { + for (unsigned int i = 0; i val-type-length; i++) { + copy_constant_array_to_storage(storage, val-array_elements[i], +idx, array_elements, boolean_true); + } + } else { + const enum glsl_base_type base_type = +val-array_elements[0]-type-base_type; + const unsigned int elements = val-array_elements[0]-type-components(); + unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int length = MIN2(val-type-length, + (storage-array_elements - *array_elements)); + + for (unsigned int i = 0; i length; i++) { + copy_constant_to_storage( storage-storage[*idx], + val-array_elements[i], + base_type, + elements, + boolean_true); + *idx += elements * dmul; + *array_elements = *array_elements + 1; + } + } +} + +void set_sampler_binding(gl_shader_program *prog, const char *name, int binding) { struct gl_uniform_storage *const storage = @@ -178,7 +210,7 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, field_constant = (ir_constant *)field_constant-next; } return; - } else if (type-is_array() type-fields.array-is_record()) { + } else if (type-without_array()-is_record()) { That's not what the old code looked for... it looked for array array-of-record. You changed it to record ||-array-of-record || array-of-array-of-record || ... . You've done this several times throughout this change. Are all of these changes safe? const glsl_type *const element_type = type-fields.array; for (unsigned int i = 0; i type-length; i++) { @@ -201,22 +233,11 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, } if (val-type-is_array()) { - const enum glsl_base_type base_type = -val-array_elements[0]-type-base_type; - const unsigned int elements = val-array_elements[0]-type-components(); unsigned int idx = 0; - unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int array_elements = 0; What is this used for? I don't see it used here or in the function (as temp shared storage). - assert(val-type-length = storage-array_elements); - for (unsigned int i = 0; i storage-array_elements; i++) { -copy_constant_to_storage( storage-storage[idx], - val-array_elements[i], - base_type, - elements, - boolean_true); - -idx += elements * dmul; - } + copy_constant_array_to_storage(storage, val, idx, + array_elements, boolean_true); } else { copy_constant_to_storage(storage-storage, val, diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp index 799c74b..654a1ca 100644 --- a/src/glsl/link_uniforms.cpp +++ b/src/glsl/link_uniforms.cpp @@ -49,8 +49,8 @@ values_for_type(const glsl_type *type) { if (type-is_sampler()) { return 1; - } else if (type-is_array() type-fields.array-is_sampler()) { - return type-array_size(); + } else if (type-is_array()) { + return type-array_size() * values_for_type(type-fields.array); } else { return type-component_slots(); } @@ -71,6 +71,7 @@ void program_resource_visitor::process(ir_variable *var) { const glsl_type *t = var-type; + const glsl_type *t_without_array = var-type-without_array(); const bool row_major = var-data.matrix_layout == GLSL_MATRIX_LAYOUT_ROW_MAJOR; @@ -136,7 +137,7 @@ program_resource_visitor::process(ir_variable *var) */ recursion(var-type, name, strlen(name), row_major, NULL, false); ralloc_free(name);
Re: [Mesa-dev] [RFC PATCH 09/12] glsl: add AoA support for linking interface blocks
On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: Adds support for linking AoA interface blocks which countain unsized arrays --- src/glsl/ir.cpp | 4 ++-- src/glsl/linker.cpp | 9 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp index 54656f8..2e1442f 100644 --- a/src/glsl/ir.cpp +++ b/src/glsl/ir.cpp @@ -1663,8 +1663,8 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name, if (type-is_interface()) this-init_interface_type(type); - else if (type-is_array() type-fields.array-is_interface()) - this-init_interface_type(type-fields.array); + else if (type-without_array()-is_interface()) + this-init_interface_type(type-without_array()); } } diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 0c44677..eb25163 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -1201,7 +1201,9 @@ public: virtual ir_visitor_status visit(ir_variable *var) { + const glsl_type *type_without_array; fixup_type(var-type, var-data.max_array_access); + type_without_array = var-type-without_array(); if (var-type-is_interface()) { if (interface_contains_unsized_arrays(var-type)) { const glsl_type *new_type = @@ -1210,11 +1212,10 @@ public: var-type = new_type; var-change_interface_type(new_type); } - } else if (var-type-is_array() - var-type-fields.array-is_interface()) { - if (interface_contains_unsized_arrays(var-type-fields.array)) { + } else if (type_without_array-is_interface()) { + if (interface_contains_unsized_arrays(type_without_array)) { const glsl_type *new_type = - resize_interface_members(var-type-fields.array, + resize_interface_members(type_without_array, var-get_max_ifc_array_access()); Will this work? I assume this max_ifc_array_access will only take one dimension into account, whereas you want several... var-change_interface_type(new_type); var-type = -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 10/15] i965/vec4: Remove broken vector size deduction in setup_builtin_uniform_values().
On Fri, Mar 20, 2015 at 7:11 AM, Francisco Jerez curroje...@riseup.net wrote: This seemed to be trying to deduce the number of uniform vector components from the parameter swizzle, but the algorithm would always give 4 as result. Instead grab the correct number of components from the GLSL type. --- Functionally, what does this affect? Are we now packing uniforms more efficiently or something? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH 10/12] glsl: add AoA support for an inteface with unsized array members
On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: Add support for setting the max access of an unsized member of an interface array of arrays. For example ifc[j][k].foo[i] where foo is unsized. --- src/glsl/ast_array_index.cpp | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp index ecef651..481bba8 100644 --- a/src/glsl/ast_array_index.cpp +++ b/src/glsl/ast_array_index.cpp @@ -64,21 +64,29 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc, } } else if (ir_dereference_record *deref_record = ir-as_dereference_record()) { - /* There are two possibilities we need to consider: + /* There are three possibilities we need to consider: * * - Accessing an element of an array that is a member of a named * interface block (e.g. ifc.foo[i]) * * - Accessing an element of an array that is a member of a named * interface block array (e.g. ifc[j].foo[i]). + * + * - Accessing an element of an array that is a member of a named + * interface block array of arrays (e.g. ifc[j][k].foo[i]). */ ir_dereference_variable *deref_var = deref_record-record-as_dereference_variable(); if (deref_var == NULL) { - if (ir_dereference_array *deref_array = - deref_record-record-as_dereference_array()) { -deref_var = deref_array-array-as_dereference_variable(); + ir_dereference_array *deref_array = +deref_record-record-as_dereference_array(); + ir_dereference_array *deref_array_prev = NULL; + while (deref_array != NULL) { +deref_array_prev = deref_array; +deref_array = deref_array-array-as_dereference_array(); } + if (deref_array_prev != NULL) +deref_var = deref_array_prev-array-as_dereference_variable(); Hm... what will this variable be? An interface, or a dereference_array? I'm guessing the latter, but the code below wants the deref_var-var to be an interface. Is that still going to work? (Don't know the code well enough... so just asking the question.) Also can you just do -array-as_dereference_array() / -as_dereference_variable() like that and expect it to work? I'd think only one of those would ever return non-null, but you end up doing both on the value pointed at by deref_array_prev-array... } if (deref_var != NULL) { -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH 12/12] glsl: Add support for lowering interface block arrays of arrays
On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: --- src/glsl/lower_named_interface_blocks.cpp | 51 ++- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/src/glsl/lower_named_interface_blocks.cpp b/src/glsl/lower_named_interface_blocks.cpp index 7304c51..8321bc1 100644 --- a/src/glsl/lower_named_interface_blocks.cpp +++ b/src/glsl/lower_named_interface_blocks.cpp @@ -65,6 +65,38 @@ #include ir_rvalue_visitor.h #include program/hash_table.h The functions below should go inside the anonymous namespace so they're not exported. Or mark them static. +const glsl_type * +process_array_type(const glsl_type *type, unsigned idx) +{ + const glsl_type *element_type = type-fields.array; + if (element_type-is_array()) { + const glsl_type *new_array_type = process_array_type(element_type, idx); + return glsl_type::get_array_instance(new_array_type, type-length); + } else { + return glsl_type::get_array_instance( +element_type-fields.structure[idx].type, type-length); + } +} + +ir_rvalue * +process_array_ir(void * const mem_ctx, + ir_dereference_array *deref_array_prev, + ir_rvalue *deref_var) +{ + ir_dereference_array *deref_array = + deref_array_prev-array-as_dereference_array(); + + if (deref_array == NULL) { + return new(mem_ctx) ir_dereference_array(deref_var, + deref_array_prev-array_index); + } else { + deref_array = (ir_dereference_array *) + process_array_ir(mem_ctx, deref_array, deref_var); + return new(mem_ctx) ir_dereference_array(deref_array, + deref_array_prev-array_index); + } +} + namespace { class flatten_named_interface_blocks_declarations : public ir_rvalue_visitor @@ -111,15 +143,9 @@ flatten_named_interface_blocks_declarations::run(exec_list *instructions) if (var-data.mode == ir_var_uniform) continue; - const glsl_type * iface_t = var-type; - const glsl_type * array_t = NULL; + const glsl_type * iface_t = var-type-without_array(); exec_node *insert_pos = var; - if (iface_t-is_array()) { - array_t = iface_t; - iface_t = array_t-fields.array; - } - assert (iface_t-is_interface()); for (unsigned i = 0; i iface_t-length; i++) { @@ -135,7 +161,7 @@ flatten_named_interface_blocks_declarations::run(exec_list *instructions) ir_variable *new_var; char *var_name = ralloc_strdup(mem_ctx, iface_t-fields.structure[i].name); -if (array_t == NULL) { +if (!var-type-is_array()) { new_var = new(mem_ctx) ir_variable(iface_t-fields.structure[i].type, var_name, @@ -143,9 +169,7 @@ flatten_named_interface_blocks_declarations::run(exec_list *instructions) new_var-data.from_named_ifc_block_nonarray = 1; } else { const glsl_type *new_array_type = - glsl_type::get_array_instance( - iface_t-fields.structure[i].type, - array_t-length); + process_array_type(var-type, i); new_var = new(mem_ctx) ir_variable(new_array_type, var_name, @@ -231,9 +255,8 @@ flatten_named_interface_blocks_declarations::handle_rvalue(ir_rvalue **rvalue) ir_dereference_array *deref_array = ir-record-as_dereference_array(); if (deref_array != NULL) { - *rvalue = -new(mem_ctx) ir_dereference_array(deref_var, - deref_array-array_index); + *rvalue = process_array_ir(mem_ctx, deref_array, +(ir_rvalue *)deref_var); } else { *rvalue = deref_var; } -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 89705] lp_bld_debug.cpp:280:54: error: no matching function for call to ‘llvm?=::TargetMachine::getSubtargetImpl()=?UTF-8?Q?’
https://bugs.freedesktop.org/show_bug.cgi?id=89705 --- Comment #1 from Dieter Nützel die...@nuetzel-hh.de --- I get this too on r600/Turks XT. /opt/mesa llc --version LLVM (http://llvm.org/): LLVM version 3.7.0svn Optimized build with assertions. Built Mar 21 2015 (16:59:53). Default target: x86_64-unknown-linux-gnu Host CPU: corei7 Registered Targets: amdgcn - AMD GCN GPUs r600 - AMD GPUs HD2XXX-HD6XXX x86- 32-bit X86: Pentium-Pro and above x86-64 - 64-bit X86: EM64T and AMD64 -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH 06/12] glsl: dont allow gl_PerVertex to be redeclared as an array of arrays
On Sat, 2015-03-21 at 19:34 -0400, Ilia Mirkin wrote: On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: Signed-off-by: Timothy Arceri t_arc...@yahoo.com.au --- src/glsl/ast_to_hir.cpp | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index 36f3eb7..7516e60 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -5595,7 +5595,11 @@ ast_interface_block::hir(exec_list *instructions, _mesa_shader_stage_to_string(state-stage)); } if (this-instance_name == NULL || - strcmp(this-instance_name, gl_in) != 0 || this-array_specifier == NULL) { + strcmp(this-instance_name, gl_in) != 0 || this-array_specifier == NULL || + (this-array_specifier-is_unsized_array + !this-array_specifier-array_dimensions.is_empty()) || Don't know a lot about AST, but does it make sense to have -is_unsized_array array_dimensions? When would that be true? Shouldn't this just be !dim.empty() without the is_unsized_array check? ast is a pain to work with. I don't recall why I wrote it that way but it should be easy to check as it was done so the piglit tests would pass. I will take another look and see if I can rework it to be a little cleaner. Otherwise I might just drop this patch for now. + (this-array_specifier-array_dimensions.tail_pred-prev != NULL + !this-array_specifier-array_dimensions.tail_pred-prev-is_head_sentinel())) { Perhaps all this would be easier to read if above you wrote foo_type *array = this-array_specifier; Not sure. Perhaps there are some helpers to make this easier to grok? _mesa_glsl_error(loc, state, gl_PerVertex input must be redeclared as gl_in[]); -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH 07/12] glsl: Add support for linking uniform arrays of arrays
On Sat, 2015-03-21 at 19:46 -0400, Ilia Mirkin wrote: On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: --- src/glsl/link_uniform_initializers.cpp | 51 -- src/glsl/link_uniforms.cpp | 28 +++ 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp index 6907384..7817314 100644 --- a/src/glsl/link_uniform_initializers.cpp +++ b/src/glsl/link_uniform_initializers.cpp @@ -100,6 +100,38 @@ copy_constant_to_storage(union gl_constant_value *storage, } void +copy_constant_array_to_storage(struct gl_uniform_storage *const storage, + const ir_constant *val, + unsigned int *idx, + unsigned int *array_elements, + unsigned int boolean_true) +{ + if (val-type-fields.array-is_array()) { + for (unsigned int i = 0; i val-type-length; i++) { + copy_constant_array_to_storage(storage, val-array_elements[i], +idx, array_elements, boolean_true); + } + } else { + const enum glsl_base_type base_type = +val-array_elements[0]-type-base_type; + const unsigned int elements = val-array_elements[0]-type-components(); + unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int length = MIN2(val-type-length, + (storage-array_elements - *array_elements)); + + for (unsigned int i = 0; i length; i++) { + copy_constant_to_storage( storage-storage[*idx], + val-array_elements[i], + base_type, + elements, + boolean_true); + *idx += elements * dmul; + *array_elements = *array_elements + 1; + } + } +} + +void set_sampler_binding(gl_shader_program *prog, const char *name, int binding) { struct gl_uniform_storage *const storage = @@ -178,7 +210,7 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, field_constant = (ir_constant *)field_constant-next; } return; - } else if (type-is_array() type-fields.array-is_record()) { + } else if (type-without_array()-is_record()) { That's not what the old code looked for... it looked for array array-of-record. You changed it to record ||-array-of-record || array-of-array-of-record || ... . You've done this several times throughout this change. Are all of these changes safe? Yeah it's safe. In each case you will see that the non array version would be found in a preceding if statement so all we are really checking for is array-of-record || array-of-array-of-record || ... const glsl_type *const element_type = type-fields.array; for (unsigned int i = 0; i type-length; i++) { @@ -201,22 +233,11 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, } if (val-type-is_array()) { - const enum glsl_base_type base_type = -val-array_elements[0]-type-base_type; - const unsigned int elements = val-array_elements[0]-type-components(); unsigned int idx = 0; - unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int array_elements = 0; What is this used for? I don't see it used here or in the function (as temp shared storage). It's used in the function to calculate the amount of room left in storage so we don't overflow if the array is to big. Maybe it should be renamed to something better storage_count? used_storage_elements? total_array_elements? Or maybe I could just add a comment to the function: /* Used to calculate the space left in storage so we don't * overflow if the array is to big. */ - assert(val-type-length = storage-array_elements); - for (unsigned int i = 0; i storage-array_elements; i++) { -copy_constant_to_storage( storage-storage[idx], - val-array_elements[i], - base_type, - elements, - boolean_true); - -idx += elements * dmul; - } + copy_constant_array_to_storage(storage, val, idx, + array_elements, boolean_true); } else { copy_constant_to_storage(storage-storage, val, diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp index 799c74b..654a1ca 100644 --- a/src/glsl/link_uniforms.cpp +++ b/src/glsl/link_uniforms.cpp @@ -49,8 +49,8 @@ values_for_type(const glsl_type *type) { if (type-is_sampler()) {
Re: [Mesa-dev] [RFC PATCH 05/12] glsl: avoid hitting assert for arrays of arrays
On Sat, 2015-03-21 at 19:30 -0400, Ilia Mirkin wrote: On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: Also add TODO comment about adding proper support Signed-off-by: Timothy Arceri t_arc...@yahoo.com.au --- src/glsl/ir_set_program_inouts.cpp | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/glsl/ir_set_program_inouts.cpp b/src/glsl/ir_set_program_inouts.cpp index e877a20..49cf85d 100644 --- a/src/glsl/ir_set_program_inouts.cpp +++ b/src/glsl/ir_set_program_inouts.cpp @@ -194,6 +194,12 @@ ir_set_program_inouts_visitor::try_mark_partial_variable(ir_variable *var, type = type-fields.array; } + /* TODO: implement proper arrays of arrays support +* for now let the caller mark whole variable as used. +*/ + if (type-is_array() type-fields.array-is_array()) + return false; + Won't this also hit the GS case even without array-of-arrays? The code just above this takes care of the non arrays-of-arrays GS case. For reference here is is: if (this-shader_stage == MESA_SHADER_GEOMETRY var-data.mode == ir_var_shader_in) { type = type-fields.array; } The subject should reflect that this is about in/out variable liveness calculation. Yes it should. Will fix. /* The code below only handles: * * - Indexing into matrices -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH 07/12] glsl: Add support for linking uniform arrays of arrays
Perhaps I'm blind, but I don't see where that array var is used beyond just being incremented. Mind pointing it out? On Mar 21, 2015 11:31 PM, Timothy Arceri t_arc...@yahoo.com.au wrote: On Sat, 2015-03-21 at 19:46 -0400, Ilia Mirkin wrote: On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: --- src/glsl/link_uniform_initializers.cpp | 51 -- src/glsl/link_uniforms.cpp | 28 +++ 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp index 6907384..7817314 100644 --- a/src/glsl/link_uniform_initializers.cpp +++ b/src/glsl/link_uniform_initializers.cpp @@ -100,6 +100,38 @@ copy_constant_to_storage(union gl_constant_value *storage, } void +copy_constant_array_to_storage(struct gl_uniform_storage *const storage, + const ir_constant *val, + unsigned int *idx, + unsigned int *array_elements, + unsigned int boolean_true) +{ + if (val-type-fields.array-is_array()) { + for (unsigned int i = 0; i val-type-length; i++) { + copy_constant_array_to_storage(storage, val-array_elements[i], +idx, array_elements, boolean_true); + } + } else { + const enum glsl_base_type base_type = +val-array_elements[0]-type-base_type; + const unsigned int elements = val-array_elements[0]-type-components(); + unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int length = MIN2(val-type-length, + (storage-array_elements - *array_elements)); + + for (unsigned int i = 0; i length; i++) { + copy_constant_to_storage( storage-storage[*idx], + val-array_elements[i], + base_type, + elements, + boolean_true); + *idx += elements * dmul; + *array_elements = *array_elements + 1; + } + } +} + +void set_sampler_binding(gl_shader_program *prog, const char *name, int binding) { struct gl_uniform_storage *const storage = @@ -178,7 +210,7 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, field_constant = (ir_constant *)field_constant-next; } return; - } else if (type-is_array() type-fields.array-is_record()) { + } else if (type-without_array()-is_record()) { That's not what the old code looked for... it looked for array array-of-record. You changed it to record ||-array-of-record || array-of-array-of-record || ... . You've done this several times throughout this change. Are all of these changes safe? Yeah it's safe. In each case you will see that the non array version would be found in a preceding if statement so all we are really checking for is array-of-record || array-of-array-of-record || ... const glsl_type *const element_type = type-fields.array; for (unsigned int i = 0; i type-length; i++) { @@ -201,22 +233,11 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, } if (val-type-is_array()) { - const enum glsl_base_type base_type = -val-array_elements[0]-type-base_type; - const unsigned int elements = val-array_elements[0]-type-components(); unsigned int idx = 0; - unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int array_elements = 0; What is this used for? I don't see it used here or in the function (as temp shared storage). It's used in the function to calculate the amount of room left in storage so we don't overflow if the array is to big. Maybe it should be renamed to something better storage_count? used_storage_elements? total_array_elements? Or maybe I could just add a comment to the function: /* Used to calculate the space left in storage so we don't * overflow if the array is to big. */ - assert(val-type-length = storage-array_elements); - for (unsigned int i = 0; i storage-array_elements; i++) { -copy_constant_to_storage( storage-storage[idx], - val-array_elements[i], - base_type, - elements, - boolean_true); - -idx += elements * dmul; - } + copy_constant_array_to_storage(storage, val, idx, + array_elements, boolean_true); } else { copy_constant_to_storage(storage-storage,
Re: [Mesa-dev] [RFC PATCH 09/12] glsl: add AoA support for linking interface blocks
On Sat, 2015-03-21 at 19:51 -0400, Ilia Mirkin wrote: On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: Adds support for linking AoA interface blocks which countain unsized arrays --- src/glsl/ir.cpp | 4 ++-- src/glsl/linker.cpp | 9 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp index 54656f8..2e1442f 100644 --- a/src/glsl/ir.cpp +++ b/src/glsl/ir.cpp @@ -1663,8 +1663,8 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name, if (type-is_interface()) this-init_interface_type(type); - else if (type-is_array() type-fields.array-is_interface()) - this-init_interface_type(type-fields.array); + else if (type-without_array()-is_interface()) + this-init_interface_type(type-without_array()); } } diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 0c44677..eb25163 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -1201,7 +1201,9 @@ public: virtual ir_visitor_status visit(ir_variable *var) { + const glsl_type *type_without_array; fixup_type(var-type, var-data.max_array_access); + type_without_array = var-type-without_array(); if (var-type-is_interface()) { if (interface_contains_unsized_arrays(var-type)) { const glsl_type *new_type = @@ -1210,11 +1212,10 @@ public: var-type = new_type; var-change_interface_type(new_type); } - } else if (var-type-is_array() - var-type-fields.array-is_interface()) { - if (interface_contains_unsized_arrays(var-type-fields.array)) { + } else if (type_without_array-is_interface()) { + if (interface_contains_unsized_arrays(type_without_array)) { const glsl_type *new_type = - resize_interface_members(var-type-fields.array, + resize_interface_members(type_without_array, var-get_max_ifc_array_access()); Will this work? I assume this max_ifc_array_access will only take one dimension into account, whereas you want several... max_ifc_array_access is just used to give the unsized dimension (which can only be outermost dimension) a size. So we only need to worry about that one dimension. var-change_interface_type(new_type); var-type = -- 2.1.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [RFC PATCH 07/12] glsl: Add support for linking uniform arrays of arrays
On Sat, 2015-03-21 at 23:35 -0400, Ilia Mirkin wrote: Perhaps I'm blind, but I don't see where that array var is used beyond just being incremented. Mind pointing it out? No problem. unsigned int length = MIN2(val-type-length, (storage-array_elements - *array_elements)); On Mar 21, 2015 11:31 PM, Timothy Arceri t_arc...@yahoo.com.au wrote: On Sat, 2015-03-21 at 19:46 -0400, Ilia Mirkin wrote: On Sat, Mar 21, 2015 at 5:49 AM, Timothy Arceri t_arc...@yahoo.com.au wrote: --- src/glsl/link_uniform_initializers.cpp | 51 -- src/glsl/link_uniforms.cpp | 28 +++ 2 files changed, 52 insertions(+), 27 deletions(-) diff --git a/src/glsl/link_uniform_initializers.cpp b/src/glsl/link_uniform_initializers.cpp index 6907384..7817314 100644 --- a/src/glsl/link_uniform_initializers.cpp +++ b/src/glsl/link_uniform_initializers.cpp @@ -100,6 +100,38 @@ copy_constant_to_storage(union gl_constant_value *storage, } void +copy_constant_array_to_storage(struct gl_uniform_storage *const storage, + const ir_constant *val, + unsigned int *idx, + unsigned int *array_elements, + unsigned int boolean_true) +{ + if (val-type-fields.array-is_array()) { + for (unsigned int i = 0; i val-type-length; i ++) { + copy_constant_array_to_storage(storage, val-array_elements[i], +idx, array_elements, boolean_true); + } + } else { + const enum glsl_base_type base_type = +val-array_elements[0]-type-base_type; + const unsigned int elements = val-array_elements[0]-type-components(); + unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int length = MIN2(val-type-length, + (storage-array_elements - *array_elements)); + + for (unsigned int i = 0; i length; i++) { + copy_constant_to_storage( storage-storage[*idx], + val-array_elements[i], + base_type, + elements, + boolean_true); + *idx += elements * dmul; + *array_elements = *array_elements + 1; + } + } +} + +void set_sampler_binding(gl_shader_program *prog, const char *name, int binding) { struct gl_uniform_storage *const storage = @@ -178,7 +210,7 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, field_constant = (ir_constant *)field_constant-next; } return; - } else if (type-is_array() type-fields.array-is_record()) { + } else if (type-without_array()-is_record()) { That's not what the old code looked for... it looked for array array-of-record. You changed it to record ||-array-of-record || array-of-array-of-record || ... . You've done this several times throughout this change. Are all of these changes safe? Yeah it's safe. In each case you will see that the non array version would be found in a preceding if statement so all we are really checking for is array-of-record || array-of-array-of-record || ... const glsl_type *const element_type = type-fields.array; for (unsigned int i = 0; i type-length; i++) { @@ -201,22 +233,11 @@ set_uniform_initializer(void *mem_ctx, gl_shader_program *prog, } if (val-type-is_array()) { - const enum glsl_base_type base_type = -val-array_elements[0]-type-base_type; - const unsigned int elements = val-array_elements[0]-type-components(); unsigned int idx = 0; - unsigned dmul = (base_type == GLSL_TYPE_DOUBLE) ? 2 : 1; + unsigned int array_elements = 0; What is this used for? I don't see it used here or in the function (as temp shared