Skip to content
Snippets Groups Projects
Commit 977db7cc authored by Ian Romanick's avatar Ian Romanick
Browse files

glsl: Rework lowering of non-constant array indexing

The previous implementation could easily get tricked if the LHS of an
assignment included a non-constant index that was "inside" another
dereference.  For example:

    mat4 m[2];
    m[0][i] = vec4(0.0);

Due to the way it tracked whether the array was being assigned, it
would think that the non-constant index was in an r-value.  The new
code fixes that by tracking l-values and r-values differently.  The
index is also replaced by cloning the IR and replacing the index
variable instead of the odd way it was done before.

v2: Apply some simplifications suggested by Eric Anholt.  Making
assignment_generator::rvalue be ir_dereference instead of ir_rvalue
simplified the code a bit.

Fixes i965 piglit fs-temp-array-mat[234]-index-wr and
vs-varying-array-mat[234]-index-wr.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=34691


Reviewed-by: default avatarEric Anholt <eric@anholt.net>
(cherry picked from commit 1731ac30)

To make bisects work, this also squashes in:

glsl: Correctly return progress from lower_variable_index_to_cond_assign

lower_variable_index_to_cond_assign runs until it can't make any more
progress.  It then returns the result of the last pass which will
always be false.  This caused the lowering loop in
_mesa_ir_link_shader to end before doing one last round of
lower_if_to_cond_assign.  This caused several if-statements (resulting
from lower_variable_index_to_cond_assign) to be left in the IR.

In addition to this change, lower_variable_index_to_cond_assign should
take a flag indicating whether or not it should even generate
if-statements.  This is easily controlled by
switch_generator::linear_sequence_max_length.  This would generate
much better code on architectures without any flow contol.

Fixes i915 piglit regressions glsl-texcoord-array and
glsl-fs-vec4-indexing-temp-src.

Reviewed-by: default avatarEric Anholt <eric@anholt.net>
(cherry picked from commit c1e591ee)
parent d31c1c33
No related branches found
No related tags found
No related merge requests found
...@@ -29,6 +29,21 @@ ...@@ -29,6 +29,21 @@
* *
* Pre-DX10 GPUs often don't have a native way to do this operation, * Pre-DX10 GPUs often don't have a native way to do this operation,
* and this works around that. * and this works around that.
*
* The lowering process proceeds as follows. Each non-constant index
* found in an r-value is converted to a canonical form \c array[i]. Each
* element of the array is conditionally assigned to a temporary by comparing
* \c i to a constant index. This is done by cloning the canonical form and
* replacing all occurances of \c i with a constant. Each remaining occurance
* of the canonical form in the IR is replaced with a dereference of the
* temporary variable.
*
* L-values with non-constant indices are handled similarly. In this case,
* the RHS of the assignment is assigned to a temporary. The non-constant
* index is replace with the canonical form (just like for r-values). The
* temporary is conditionally assigned to each element of the canonical form
* by comparing \c i with each index. The same clone-and-replace scheme is
* used.
*/ */
#include "ir.h" #include "ir.h"
...@@ -43,10 +58,70 @@ is_array_or_matrix(const ir_instruction *ir) ...@@ -43,10 +58,70 @@ is_array_or_matrix(const ir_instruction *ir)
return (ir->type->is_array() || ir->type->is_matrix()); return (ir->type->is_array() || ir->type->is_matrix());
} }
/**
* Replace a dereference of a variable with a specified r-value
*
* Each time a dereference of the specified value is replaced, the r-value
* tree is cloned.
*/
class deref_replacer : public ir_rvalue_visitor {
public:
deref_replacer(const ir_variable *variable_to_replace, ir_rvalue *value)
: variable_to_replace(variable_to_replace), value(value),
progress(false)
{
assert(this->variable_to_replace != NULL);
assert(this->value != NULL);
}
virtual void handle_rvalue(ir_rvalue **rvalue)
{
ir_dereference_variable *const dv = (*rvalue)->as_dereference_variable();
if ((dv != NULL) && (dv->var == this->variable_to_replace)) {
this->progress = true;
*rvalue = this->value->clone(ralloc_parent(*rvalue), NULL);
}
}
const ir_variable *variable_to_replace;
ir_rvalue *value;
bool progress;
};
/**
* Find a variable index dereference of an array in an rvalue tree
*/
class find_variable_index : public ir_hierarchical_visitor {
public:
find_variable_index()
: deref(NULL)
{
/* empty */
}
virtual ir_visitor_status visit_enter(ir_dereference_array *ir)
{
if (is_array_or_matrix(ir->array)
&& (ir->array_index->as_constant() == NULL)) {
this->deref = ir;
return visit_stop;
}
return visit_continue;
}
/**
* First array dereference found in the tree that has a non-constant index.
*/
ir_dereference_array *deref;
};
struct assignment_generator struct assignment_generator
{ {
ir_instruction* base_ir; ir_instruction* base_ir;
ir_rvalue* array; ir_dereference *rvalue;
ir_variable *old_index;
bool is_write; bool is_write;
unsigned int write_mask; unsigned int write_mask;
ir_variable* var; ir_variable* var;
...@@ -61,18 +136,23 @@ struct assignment_generator ...@@ -61,18 +136,23 @@ struct assignment_generator
* underlying variable. * underlying variable.
*/ */
void *mem_ctx = ralloc_parent(base_ir); void *mem_ctx = ralloc_parent(base_ir);
ir_dereference *element =
new(mem_ctx) ir_dereference_array(this->array->clone(mem_ctx, NULL),
new(mem_ctx) ir_constant(i));
ir_rvalue *variable = new(mem_ctx) ir_dereference_variable(this->var);
ir_assignment *assignment; /* Clone the old r-value in its entirety. Then replace any occurances of
if (is_write) { * the old variable index with the new constant index.
assignment = new(mem_ctx) ir_assignment(element, variable, condition, */
write_mask); ir_dereference *element = this->rvalue->clone(mem_ctx, NULL);
} else { ir_constant *const index = new(mem_ctx) ir_constant(i);
assignment = new(mem_ctx) ir_assignment(variable, element, condition); deref_replacer r(this->old_index, index);
} element->accept(&r);
assert(r.progress);
/* Generate a conditional assignment to (or from) the constant indexed
* array dereference.
*/
ir_rvalue *variable = new(mem_ctx) ir_dereference_variable(this->var);
ir_assignment *const assignment = (is_write)
? new(mem_ctx) ir_assignment(element, variable, condition, write_mask)
: new(mem_ctx) ir_assignment(variable, element, condition);
list->push_tail(assignment); list->push_tail(assignment);
} }
...@@ -274,7 +354,8 @@ public: ...@@ -274,7 +354,8 @@ public:
} }
ir_variable *convert_dereference_array(ir_dereference_array *orig_deref, ir_variable *convert_dereference_array(ir_dereference_array *orig_deref,
ir_assignment* orig_assign) ir_assignment* orig_assign,
ir_dereference *orig_base)
{ {
assert(is_array_or_matrix(orig_deref->array)); assert(is_array_or_matrix(orig_deref->array));
...@@ -320,9 +401,12 @@ public: ...@@ -320,9 +401,12 @@ public:
new(mem_ctx) ir_assignment(lhs, orig_deref->array_index, NULL); new(mem_ctx) ir_assignment(lhs, orig_deref->array_index, NULL);
base_ir->insert_before(assign); base_ir->insert_before(assign);
orig_deref->array_index = lhs->clone(mem_ctx, NULL);
assignment_generator ag; assignment_generator ag;
ag.array = orig_deref->array; ag.rvalue = orig_base;
ag.base_ir = base_ir; ag.base_ir = base_ir;
ag.old_index = index;
ag.var = var; ag.var = var;
if (orig_assign) { if (orig_assign) {
ag.is_write = true; ag.is_write = true;
...@@ -342,12 +426,16 @@ public: ...@@ -342,12 +426,16 @@ public:
virtual void handle_rvalue(ir_rvalue **pir) virtual void handle_rvalue(ir_rvalue **pir)
{ {
if (this->in_assignee)
return;
if (!*pir) if (!*pir)
return; return;
ir_dereference_array* orig_deref = (*pir)->as_dereference_array(); ir_dereference_array* orig_deref = (*pir)->as_dereference_array();
if (needs_lowering(orig_deref)) { if (needs_lowering(orig_deref)) {
ir_variable* var = convert_dereference_array(orig_deref, 0); ir_variable *var =
convert_dereference_array(orig_deref, NULL, orig_deref);
assert(var); assert(var);
*pir = new(ralloc_parent(base_ir)) ir_dereference_variable(var); *pir = new(ralloc_parent(base_ir)) ir_dereference_variable(var);
this->progress = true; this->progress = true;
...@@ -359,10 +447,11 @@ public: ...@@ -359,10 +447,11 @@ public:
{ {
ir_rvalue_visitor::visit_leave(ir); ir_rvalue_visitor::visit_leave(ir);
ir_dereference_array *orig_deref = ir->lhs->as_dereference_array(); find_variable_index f;
ir->lhs->accept(&f);
if (needs_lowering(orig_deref)) { if ((f.deref != NULL) && storage_type_needs_lowering(f.deref)) {
convert_dereference_array(orig_deref, ir); convert_dereference_array(f.deref, ir, ir->lhs);
ir->remove(); ir->remove();
this->progress = true; this->progress = true;
} }
...@@ -383,7 +472,17 @@ lower_variable_index_to_cond_assign(exec_list *instructions, ...@@ -383,7 +472,17 @@ lower_variable_index_to_cond_assign(exec_list *instructions,
lower_temp, lower_temp,
lower_uniform); lower_uniform);
visit_list_elements(&v, instructions); /* Continue lowering until no progress is made. If there are multiple
* levels of indirection (e.g., non-constant indexing of array elements and
return v.progress; * matrix columns of an array of matrix), each pass will only lower one
* level of indirection.
*/
bool progress_ever = false;
do {
v.progress = false;
visit_list_elements(&v, instructions);
progress_ever = v.progress || progress_ever;
} while (v.progress);
return progress_ever;
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment