<div dir="ltr">On 23 October 2013 03:31, Timothy Arceri <span dir="ltr"><<a href="mailto:t_arceri@yahoo.com.au" target="_blank">t_arceri@yahoo.com.au</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>The main purpose of this patch is to increase readability of<br>
</div>the array code by introducing is_unsized_array() to glsl_types.<br>
Some redundent is_array() checks are also removed, and small number<br>
of other related clean ups.<br>
<div><br>
The introduction of is_unsized_array() should also make the<br>
ARB_arrays_of_arrays code simpler and more readable when it arrives.<br>
<br>
</div>V2: Also replace code that checks for unsized arrays directly with the<br>
length variable<br>
<div><br>
Signed-off-by: Timothy Arceri <<a href="mailto:t_arceri@yahoo.com.au" target="_blank">t_arceri@yahoo.com.au</a>><br></div></blockquote><div><br></div><div>Nice clean-up, thanks. With the minor fixes noted below, both patches are:<br>
<br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>><br><br>I've pushed them upstream.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
---<br>
src/glsl/ast_array_index.cpp | 2 +-<br>
</div> src/glsl/ast_function.cpp | 11 +++++------<br>
src/glsl/ast_to_hir.cpp | 24 ++++++++++--------------<br>
<div> src/glsl/glsl_types.h | 11 ++++++++---<br>
src/glsl/hir_field_selection.cpp | 2 +-<br>
</div> src/glsl/linker.cpp | 4 ++--<br>
src/glsl/opt_array_splitting.cpp | 2 +-<br>
7 files changed, 28 insertions(+), 28 deletions(-)<br>
<div><br>
diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp<br>
index 107c29e..f7b5e83 100644<br>
--- a/src/glsl/ast_array_index.cpp<br>
+++ b/src/glsl/ast_array_index.cpp<br>
@@ -165,7 +165,7 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,<br>
if (array->type->is_array())<br>
update_max_array_access(array, idx, &loc, state);<br>
} else if (const_index == NULL && array->type->is_array()) {<br>
- if (array->type->array_size() == 0) {<br>
+ if (array->type->is_unsized_array()) {<br>
_mesa_glsl_error(&loc, state, "unsized array index must be constant");<br>
} else if (array->type->fields.array->is_interface()<br>
&& array->variable_referenced()->mode == ir_var_uniform) {<br>
</div>diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp<br>
index 02aad4f..5efd691 100644<br>
--- a/src/glsl/ast_function.cpp<br>
+++ b/src/glsl/ast_function.cpp<br>
@@ -732,21 +732,20 @@ process_array_constructor(exec_list *instructions,<br>
exec_list actual_parameters;<br>
const unsigned parameter_count =<br>
process_parameters(instructions, &actual_parameters, parameters, state);<br>
+ bool is_unsized_array = constructor_type->is_unsized_array();<br>
<br>
- if ((parameter_count == 0)<br>
- || ((constructor_type->length != 0)<br>
+ if ((parameter_count == 0) || (!is_unsized_array<br>
&& (constructor_type->length != parameter_count))) {<br></blockquote><div><br></div><div>Formatting nit pick: where possible we try to break lines where the parenthesis nesting is as little as possible, so that we can line up the text after the line break at the column after the enclosing opening paren. I've reformmated this to:<br>
<br> if ((parameter_count == 0) ||<br> (!is_unsized_array && (constructor_type->length != parameter_count))) {<br><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- const unsigned min_param = (constructor_type->length == 0)<br>
- ? 1 : constructor_type->length;<br>
+ const unsigned min_param = is_unsized_array ? 1 : constructor_type->length;<br></blockquote><div><br></div><div>Formatting nit pick: we try to keep to <80 columns. I've reformatted this to:<br><br> const unsigned min_param = is_unsized_array<br>
? 1 : constructor_type->length;<br><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
_mesa_glsl_error(loc, state, "array constructor must have %s %u "<br>
"parameter%s",<br>
- (constructor_type->length == 0) ? "at least" : "exactly",<br>
+ is_unsized_array ? "at least" : "exactly",<br>
min_param, (min_param <= 1) ? "" : "s");<br>
return ir_rvalue::error_value(ctx);<br>
}<br>
<br>
- if (constructor_type->length == 0) {<br>
+ if (is_unsized_array) {<br>
constructor_type =<br>
glsl_type::get_array_instance(constructor_type->element_type(),<br>
parameter_count);<br>
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp<br>
index 12be2fd..abe6059 100644<br>
<div><div>--- a/src/glsl/ast_to_hir.cpp<br>
+++ b/src/glsl/ast_to_hir.cpp<br>
@@ -651,16 +651,15 @@ validate_assignment(struct _mesa_glsl_parse_state *state,<br>
if (rhs->type == lhs_type)<br>
return rhs;<br>
<br>
- /* If the array element types are the same and the size of the LHS is zero,<br>
+ /* If the array element types are the same and the LHS is unsized,<br>
* the assignment is okay for initializers embedded in variable<br>
* declarations.<br>
*<br>
* Note: Whole-array assignments are not permitted in GLSL 1.10, but this<br>
* is handled by ir_dereference::is_lvalue.<br>
*/<br>
- if (is_initializer && lhs_type->is_array() && rhs->type->is_array()<br>
- && (lhs_type->element_type() == rhs->type->element_type())<br>
- && (lhs_type->array_size() == 0)) {<br>
+ if (is_initializer && lhs_type->is_unsized_array() && rhs->type->is_array()<br>
+ && (lhs_type->element_type() == rhs->type->element_type())) {<br>
return rhs;<br>
}<br>
<br>
@@ -767,7 +766,7 @@ do_assignment(exec_list *instructions, struct _mesa_glsl_parse_state *state,<br>
* dereference of a variable. Any other case would require that the LHS<br>
* is either not an l-value or not a whole array.<br>
*/<br>
- if (lhs->type->array_size() == 0) {<br>
+ if (lhs->type->is_unsized_array()) {<br>
ir_dereference *const d = lhs->as_dereference();<br>
<br>
assert(d != NULL);<br>
@@ -2355,8 +2354,7 @@ get_variable_being_redeclared(ir_variable *var, YYLTYPE loc,<br>
* later re-declare the same name as an array of the same<br>
* type and specify a size."<br>
*/<br>
- if ((earlier->type->array_size() == 0)<br>
- && var->type->is_array()<br>
+ if (earlier->type->is_unsized_array() && var->type->is_array()<br>
&& (var->type->element_type() == earlier->type->element_type())) {<br>
/* FINISHME: This doesn't match the qualifiers on the two<br>
* FINISHME: declarations. It's not 100% clear whether this is<br>
</div></div>@@ -2608,7 +2606,7 @@ handle_geometry_shader_input_decl(struct _mesa_glsl_parse_state *state,<br>
return;<br>
}<br>
<br>
- if (var->type->length == 0) {<br>
+ if (var->type->is_unsized_array()) {<br>
/* Section 4.3.8.1 (Input Layout Qualifiers) of the GLSL 1.50 spec says:<br>
*<br>
* All geometry shader input unsized array declarations will be<br>
@@ -3257,7 +3255,7 @@ ast_declarator_list::hir(exec_list *instructions,<br>
const glsl_type *const t = (earlier == NULL)<br>
? var->type : earlier->type;<br>
<br>
- if (t->is_array() && t->length == 0)<br>
+ if (t->is_unsized_array())<br>
/* Section 10.17 of the GLSL ES 1.00 specification states that<br>
* unsized array declarations have been removed from the language.<br>
* Arrays that are sized using an initializer are still explicitly<br>
<div>@@ -3390,7 +3388,7 @@ ast_parameter_declarator::hir(exec_list *instructions,<br>
type = process_array_type(&loc, type, this->array_size, state);<br>
}<br>
<br>
- if (!type->is_error() && type->array_size() == 0) {<br>
+ if (!type->is_error() && type->is_unsized_array()) {<br>
_mesa_glsl_error(&loc, state, "arrays passed as parameters must have "<br>
"a declared size");<br>
type = glsl_type::error_type;<br>
</div>@@ -3562,7 +3560,7 @@ ast_function::hir(exec_list *instructions,<br>
* "Arrays are allowed as arguments and as the return type. In both<br>
* cases, the array must be explicitly sized."<br>
*/<br>
- if (return_type->is_array() && return_type->length == 0) {<br>
+ if (return_type->is_unsized_array()) {<br>
YYLTYPE loc = this->get_location();<br>
_mesa_glsl_error(& loc, state,<br>
"function `%s' return type array must be explicitly "<br>
@@ -5016,10 +5014,8 @@ ast_gs_input_layout::hir(exec_list *instructions,<br>
/* Note: gl_PrimitiveIDIn has mode ir_var_shader_in, but it's not an<br>
* array; skip it.<br>
*/<br>
- if (!var->type->is_array())<br>
- continue;<br>
<br>
- if (var->type->length == 0) {<br>
+ if (var->type->is_unsized_array()) {<br>
if (var->max_array_access >= num_vertices) {<br>
_mesa_glsl_error(&loc, state,<br>
"this geometry shader input layout implies %u"<br>
diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h<br>
index e60c191..177d863 100644<br>
<div>--- a/src/glsl/glsl_types.h<br>
+++ b/src/glsl/glsl_types.h<br>
@@ -468,7 +468,6 @@ struct glsl_type {<br>
: error_type;<br>
}<br>
<br>
-<br>
/**<br>
* Get the type of a structure field<br>
*<br>
@@ -478,13 +477,11 @@ struct glsl_type {<br>
*/<br>
const glsl_type *field_type(const char *name) const;<br>
<br>
-<br>
/**<br>
* Get the location of a filed within a record type<br>
*/<br>
int field_index(const char *name) const;<br>
<br>
-<br>
/**<br>
* Query the number of elements in an array type<br>
*<br></div></blockquote><div><br></div><div>We try to keep whitespace cleanups in separate patches. I've moved these to their own patch.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
@@ -499,6 +496,14 @@ struct glsl_type {<br>
}<br>
<br>
/**<br>
</div>+ * Query whether the array size for all dimensions has been declared.<br>
<div>+ */<br>
+ bool is_unsized_array() const<br>
+ {<br>
+ return is_array() && length == 0;<br>
+ }<br>
+<br>
+ /**<br>
* Return the number of coordinate components needed for this sampler type.<br>
*<br>
* This is based purely on the sampler's dimensionality. For example, this<br>
diff --git a/src/glsl/hir_field_selection.cpp b/src/glsl/hir_field_selection.cpp<br>
index 08be743..1e92c89 100644<br>
--- a/src/glsl/hir_field_selection.cpp<br>
+++ b/src/glsl/hir_field_selection.cpp<br>
@@ -72,7 +72,7 @@ _mesa_ast_field_selection_to_hir(const ast_expression *expr,<br>
_mesa_glsl_error(&loc, state, "length method takes no arguments");<br>
<br>
if (op->type->is_array()) {<br>
- if (op->type->array_size() == 0)<br>
+ if (op->type->is_unsized_array())<br>
_mesa_glsl_error(&loc, state, "length called on unsized array");<br>
<br>
result = new(ctx) ir_constant(op->type->array_size());<br>
</div>diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp<br>
index 0a949b4..01b7a60 100644<br>
--- a/src/glsl/linker.cpp<br>
+++ b/src/glsl/linker.cpp<br>
@@ -1105,7 +1105,7 @@ private:<br>
*/<br>
static void fixup_type(const glsl_type **type, unsigned max_array_access)<br>
{<br>
- if ((*type)->is_array() && (*type)->length == 0) {<br>
+ if ((*type)->is_unsized_array()) {<br>
*type = glsl_type::get_array_instance((*type)->fields.array,<br>
max_array_access + 1);<br>
assert(*type != NULL);<br>
@@ -1120,7 +1120,7 @@ private:<br>
{<br>
for (unsigned i = 0; i < type->length; i++) {<br>
const glsl_type *elem_type = type->fields.structure[i].type;<br>
- if (elem_type->is_array() && elem_type->length == 0)<br>
+ if (elem_type->is_unsized_array())<br>
return true;<br>
}<br>
return false;<br>
diff --git a/src/glsl/opt_array_splitting.cpp b/src/glsl/opt_array_splitting.cpp<br>
index 34ac836..c7c5f67 100644<br>
--- a/src/glsl/opt_array_splitting.cpp<br>
+++ b/src/glsl/opt_array_splitting.cpp<br>
@@ -132,7 +132,7 @@ ir_array_reference_visitor::get_variable_entry(ir_variable *var)<br>
/* If the array hasn't been sized yet, we can't split it. After<br>
* linking, this should be resolved.<br>
*/<br>
- if (var->type->is_array() && var->type->length == 0)<br>
+ if (var->type->is_unsized_array())<br>
return NULL;<br>
<br>
foreach_iter(exec_list_iterator, iter, this->variable_list) {<br>
<div><div>--<br>
1.8.3.1<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>