[Mesa-dev] [PATCH 3/4] glsl/gs: handle gl_ClipDistance geometry input in lower_clip_distance.
Paul Berry
stereotype441 at gmail.com
Sat Oct 5 10:25:28 PDT 2013
On 28 September 2013 14:59, Paul Berry <stereotype441 at gmail.com> wrote:
> On 27 September 2013 14:32, Ian Romanick <idr at freedesktop.org> wrote:
>
>> There are some bits of this patch I'm trying to understand. I think
>> they can be cleared up by one question below...
>>
>> On 09/14/2013 01:00 PM, Paul Berry wrote:
>> > From: Bryan Cain <bryancain3 at gmail.com>
>> >
>> > This corresponds to the lowering of gl_ClipDistance to
>> > gl_ClipDistanceMESA for vertex and geometry shader outputs. Since
>> > this lowering pass occurs after lower_named_interface blocks, it deals
>> > with 2D arrays (gl_ClipDistance[vertex][clip_plane]) rather than 1D
>> > arrays in an interface block
>> > (gl_in[vertex].gl_ClipDistance[clip_plane]).
>> >
>> > v2 (Paul Berry <stereotype441 at gmail.com>): Fix indexing order for
>> > gl_ClipDistance input lowering. Properly lower bulk assignment of
>> > gl_ClipDistance inputs. Rework for GLSL 1.50 style geometry shaders.
>> > ---
>> > src/glsl/lower_clip_distance.cpp | 223
>> +++++++++++++++++++++++++++++----------
>> > 1 file changed, 165 insertions(+), 58 deletions(-)
>> >
>> > diff --git a/src/glsl/lower_clip_distance.cpp
>> b/src/glsl/lower_clip_distance.cpp
>> > index d6cf944..16eaef6 100644
>> > --- a/src/glsl/lower_clip_distance.cpp
>> > +++ b/src/glsl/lower_clip_distance.cpp
>> > @@ -53,13 +53,16 @@
>> > class lower_clip_distance_visitor : public ir_rvalue_visitor {
>> > public:
>> > lower_clip_distance_visitor()
>> > - : progress(false), old_clip_distance_var(NULL),
>> > - new_clip_distance_var(NULL)
>> > + : progress(false), old_clip_distance_1d_var(NULL),
>> > + old_clip_distance_2d_var(NULL), new_clip_distance_1d_var(NULL),
>> > + new_clip_distance_2d_var(NULL)
>> > {
>> > }
>> >
>> > virtual ir_visitor_status visit(ir_variable *);
>> > void create_indices(ir_rvalue*, ir_rvalue *&, ir_rvalue *&);
>> > + bool is_clip_distance_vec8(ir_rvalue *ir);
>> > + ir_rvalue *lower_clip_distance_vec8(ir_rvalue *ir);
>> > virtual ir_visitor_status visit_leave(ir_assignment *);
>> > void visit_new_assignment(ir_assignment *ir);
>> > virtual ir_visitor_status visit_leave(ir_call *);
>> > @@ -73,12 +76,14 @@ public:
>> > /**
>> > * Pointer to the declaration of gl_ClipDistance, if found.
>> > */
>> > - ir_variable *old_clip_distance_var;
>> > + ir_variable *old_clip_distance_1d_var;
>> > + ir_variable *old_clip_distance_2d_var;
>> >
>> > /**
>> > * Pointer to the newly-created gl_ClipDistanceMESA variable.
>> > */
>> > - ir_variable *new_clip_distance_var;
>> > + ir_variable *new_clip_distance_1d_var;
>> > + ir_variable *new_clip_distance_2d_var;
>> > };
>> >
>> >
>> > @@ -89,30 +94,60 @@ public:
>> > ir_visitor_status
>> > lower_clip_distance_visitor::visit(ir_variable *ir)
>> > {
>> > - /* No point in looking for the declaration of gl_ClipDistance if
>> > - * we've already found it.
>> > - */
>> > - if (this->old_clip_distance_var)
>> > + if (!ir->name || strcmp(ir->name, "gl_ClipDistance") != 0)
>> > return visit_continue;
>> > + assert (ir->type->is_array());
>> > +
>> > + if (!ir->type->element_type()->is_array()) {
>> > + /* 1D gl_ClipDistance (used for vertex and geometry output, and
>> fragment
>> > + * input).
>> > + */
>> > + if (this->old_clip_distance_1d_var)
>> > + return visit_continue;
>> >
>> > - if (ir->name && strcmp(ir->name, "gl_ClipDistance") == 0) {
>> > this->progress = true;
>> > - this->old_clip_distance_var = ir;
>> > - assert (ir->type->is_array());
>> > + this->old_clip_distance_1d_var = ir;
>> > assert (ir->type->element_type() == glsl_type::float_type);
>> > unsigned new_size = (ir->type->array_size() + 3) / 4;
>> >
>> > /* Clone the old var so that we inherit all of its properties */
>> > - this->new_clip_distance_var = ir->clone(ralloc_parent(ir), NULL);
>> > + this->new_clip_distance_1d_var = ir->clone(ralloc_parent(ir),
>> NULL);
>> >
>> > /* And change the properties that we need to change */
>> > - this->new_clip_distance_var->name
>> > - = ralloc_strdup(this->new_clip_distance_var,
>> "gl_ClipDistanceMESA");
>> > - this->new_clip_distance_var->type
>> > + this->new_clip_distance_1d_var->name
>> > + = ralloc_strdup(this->new_clip_distance_1d_var,
>> > + "gl_ClipDistanceMESA");
>> > + this->new_clip_distance_1d_var->type
>> > = glsl_type::get_array_instance(glsl_type::vec4_type,
>> new_size);
>> > - this->new_clip_distance_var->max_array_access =
>> ir->max_array_access / 4;
>> > + this->new_clip_distance_1d_var->max_array_access
>> > + = ir->max_array_access / 4;
>> > +
>> > + ir->replace_with(this->new_clip_distance_1d_var);
>> > + } else {
>> > + /* 2D gl_ClipDistance (used for geometry input). */
>> > + assert(ir->mode == ir_var_shader_in);
>> > + if (this->old_clip_distance_2d_var)
>> > + return visit_continue;
>> >
>> > - ir->replace_with(this->new_clip_distance_var);
>> > + this->progress = true;
>> > + this->old_clip_distance_2d_var = ir;
>> > + assert (ir->type->element_type()->element_type() ==
>> glsl_type::float_type);
>> > + unsigned new_size = (ir->type->element_type()->array_size() + 3)
>> / 4;
>> > +
>> > + /* Clone the old var so that we inherit all of its properties */
>> > + this->new_clip_distance_2d_var = ir->clone(ralloc_parent(ir),
>> NULL);
>> > +
>> > + /* And change the properties that we need to change */
>> > + this->new_clip_distance_2d_var->name
>> > + = ralloc_strdup(this->new_clip_distance_2d_var,
>> "gl_ClipDistanceMESA");
>> > + this->new_clip_distance_2d_var->type =
>> glsl_type::get_array_instance(
>> > + glsl_type::get_array_instance(glsl_type::vec4_type,
>> > + new_size),
>> > + ir->type->array_size());
>> > + this->new_clip_distance_2d_var->max_array_access
>> > + = ir->max_array_access / 4;
>> > +
>> > + ir->replace_with(this->new_clip_distance_2d_var);
>> > }
>> > return visit_continue;
>> > }
>> > @@ -177,39 +212,99 @@
>> lower_clip_distance_visitor::create_indices(ir_rvalue *old_index,
>> > }
>> >
>> >
>> > +/**
>> > + * Determine whether the given rvalue describes an array of 8 floats
>> that
>> > + * needs to be lowered to an array of 2 vec4's; that is, determine
>> whether it
>> > + * matches one of the following patterns:
>> > + *
>> > + * - gl_ClipDistance (if gl_ClipDistance is 1D)
>> > + * - gl_ClipDistance[i] (if gl_ClipDistance is 2D)
>> > + */
>> > +bool
>> > +lower_clip_distance_visitor::is_clip_distance_vec8(ir_rvalue *ir)
>> > +{
>>
>> Is it ever possible for both of these to be taken?
>>
>
> Yes. If we are in a geometry shader, then there's both a 1D clip distance
> variable (for the gl_ClipDistance output) and a 2D clip distance variable
> (for the gl_in[i].gl_ClipDistance input). Note that this pass runs after
> lower_named_interface_blocks(), so the clip distance input just looks like
> a bare 2D array. So if, for example, ir represents the gl_ClipDistance[i]
> part of the gl_ClipDistance[i][j] input, then:
>
>
>>
>> > + if (this->old_clip_distance_1d_var) {
>>
>
> This branch will be taken
>
>
>> > + ir_dereference_variable *var_ref =
>> ir->as_dereference_variable();
>>
>
> But var_ref will be NULL because ir is not a variable dereference, so the
> following if test will skip:
>
>
>> > + if (var_ref && var_ref->var == this->old_clip_distance_1d_var)
>> > + return true;
>> > + }
>>
>
> Then we'll fall through to here
>
>
>> > + if (this->old_clip_distance_2d_var) {
>>
>
> And this branch will be taken.
>
>
>> > + ir_dereference_array *array_ref = ir->as_dereference_array();
>>
>
> Now array_ref will be non-NULL because the ir is an array dereference.
>
>
>> > + if (array_ref) {
>> > + ir_dereference_variable *var_ref =
>> > + array_ref->array->as_dereference_variable();
>>
>
> So then we'll get here and var_ref will be a reference to the
> gl_ClipDistance (2D) input variable.
>
>
>> > + if (var_ref && var_ref->var == this->old_clip_distance_2d_var)
>> > + return true;
>> > + }
>> > + }
>> > + return false;
>> > +}
>>
>
> Hopefully that helped?
>
>>
>> > +
>> > +
>> > +/**
>> > + * If the given ir satisfies is_clip_distance_vec8(), return new ir
>> > + * representing its lowered equivalent. That is, map:
>> > + *
>> > + * - gl_ClipDistance => gl_ClipDistanceMESA (if gl_ClipDistance
>> is 1D)
>> > + * - gl_ClipDistance[i] => gl_ClipDistanceMESA[i] (if gl_ClipDistance
>> is 2D)
>> > + *
>> > + * Otherwise return NULL.
>> > + */
>> > +ir_rvalue *
>> > +lower_clip_distance_visitor::lower_clip_distance_vec8(ir_rvalue *ir)
>> > +{
>> > + if (this->old_clip_distance_1d_var) {
>> > + ir_dereference_variable *var_ref = ir->as_dereference_variable();
>> > + if (var_ref && var_ref->var == this->old_clip_distance_1d_var) {
>> > + return new(ralloc_parent(ir))
>> > + ir_dereference_variable(this->new_clip_distance_1d_var);
>> > + }
>> > + }
>> > + if (this->old_clip_distance_2d_var) {
>> > + ir_dereference_array *array_ref = ir->as_dereference_array();
>> > + if (array_ref) {
>> > + ir_dereference_variable *var_ref =
>> > + array_ref->array->as_dereference_variable();
>> > + if (var_ref && var_ref->var ==
>> this->old_clip_distance_2d_var) {
>> > + return new(ralloc_parent(ir))
>> > + ir_dereference_array(this->new_clip_distance_2d_var,
>> > + array_ref->array_index);
>> > + }
>> > + }
>> > + }
>> > + return NULL;
>> > +}
>> > +
>> > +
>> > void
>> > lower_clip_distance_visitor::handle_rvalue(ir_rvalue **rv)
>> > {
>> > - /* If the gl_ClipDistance var hasn't been declared yet, then
>> > - * there's no way this deref can refer to it.
>> > - */
>> > - if (!this->old_clip_distance_var || *rv == NULL)
>> > + if (*rv == NULL)
>> > return;
>> >
>> > ir_dereference_array *const array_deref =
>> (*rv)->as_dereference_array();
>> > if (array_deref == NULL)
>> > return;
>> >
>> > - /* Replace any expression that indexes into the gl_ClipDistance
>> array
>> > + /* Replace any expression that indexes one of the floats in
>> gl_ClipDistance
>> > * with an expression that indexes into one of the vec4's in
>> > * gl_ClipDistanceMESA and accesses the appropriate component.
>> > */
>> > - ir_dereference_variable *old_var_ref =
>> > - array_deref->array->as_dereference_variable();
>> > - if (old_var_ref && old_var_ref->var == this->old_clip_distance_var)
>> {
>> > + ir_rvalue *lowered_vec8 =
>> > + this->lower_clip_distance_vec8(array_deref->array);
>> > + if (lowered_vec8 != NULL) {
>> > this->progress = true;
>> > ir_rvalue *array_index;
>> > ir_rvalue *swizzle_index;
>> > this->create_indices(array_deref->array_index, array_index,
>> swizzle_index);
>> > void *mem_ctx = ralloc_parent(array_deref);
>> >
>> > - ir_dereference_array *const ClipDistanceMESA_deref =
>> > - new(mem_ctx) ir_dereference_array(this->new_clip_distance_var,
>> > - array_index);
>> > + ir_dereference_array *const new_array_deref =
>> > + new(mem_ctx) ir_dereference_array(lowered_vec8, array_index);
>> >
>> > ir_expression *const expr =
>> > new(mem_ctx) ir_expression(ir_binop_vector_extract,
>> > - ClipDistanceMESA_deref,
>> > + new_array_deref,
>> > swizzle_index);
>> >
>> > *rv = expr;
>> > @@ -243,30 +338,34 @@
>> lower_clip_distance_visitor::fix_lhs(ir_assignment *ir)
>> > }
>> >
>> > /**
>> > - * Replace any assignment having gl_ClipDistance (undereferenced) as
>> its LHS
>> > - * or RHS with a sequence of assignments, one for each component of
>> the array.
>> > - * Each of these assignments is lowered to refer to
>> gl_ClipDistanceMESA as
>> > - * appropriate.
>> > + * Replace any assignment having the 1D gl_ClipDistance
>> (undereferenced) as
>> > + * its LHS or RHS with a sequence of assignments, one for each
>> component of
>> > + * the array. Each of these assignments is lowered to refer to
>> > + * gl_ClipDistanceMESA as appropriate.
>> > + *
>> > + * We need to do a similar replacement for 2D gl_ClipDistance, however
>> since
>> > + * it's an input, the only case we need to address is where a 1D slice
>> of it
>> > + * is the entire RHS of an assignment, e.g.:
>> > + *
>> > + * foo = gl_in[i].gl_ClipDistance
>> > */
>> > ir_visitor_status
>> > lower_clip_distance_visitor::visit_leave(ir_assignment *ir)
>> > {
>> > - ir_dereference_variable *lhs_var =
>> ir->lhs->as_dereference_variable();
>> > - ir_dereference_variable *rhs_var =
>> ir->rhs->as_dereference_variable();
>> > - if ((lhs_var && lhs_var->var == this->old_clip_distance_var)
>> > - || (rhs_var && rhs_var->var == this->old_clip_distance_var)) {
>> > - /* LHS or RHS of the assignment is the entire gl_ClipDistance
>> array.
>> > - * Since we are reshaping gl_ClipDistance from an array of
>> floats to an
>> > - * array of vec4's, this isn't going to work as a bulk assignment
>> > - * anymore, so unroll it to element-by-element assignments and
>> lower
>> > - * each of them.
>> > + if (this->is_clip_distance_vec8(ir->lhs) ||
>> > + this->is_clip_distance_vec8(ir->rhs)) {
>> > + /* LHS or RHS of the assignment is the entire 1D gl_ClipDistance
>> array
>> > + * (or a 1D slice of a 2D gl_ClipDistance input array). Since
>> we are
>> > + * reshaping gl_ClipDistance from an array of floats to an array
>> of
>> > + * vec4's, this isn't going to work as a bulk assignment
>> anymore, so
>> > + * unroll it to element-by-element assignments and lower each of
>> them.
>> > *
>> > * Note: to unroll into element-by-element assignments, we need
>> to make
>> > * clones of the LHS and RHS. This is safe because expressions
>> and
>> > * l-values are side-effect free.
>> > */
>> > void *ctx = ralloc_parent(ir);
>> > - int array_size = this->old_clip_distance_var->type->array_size();
>> > + int array_size = ir->lhs->type->array_size();
>> > for (int i = 0; i < array_size; ++i) {
>> > ir_dereference_array *new_lhs = new(ctx) ir_dereference_array(
>> > ir->lhs->clone(ctx, NULL), new(ctx) ir_constant(i));
>> > @@ -326,11 +425,17 @@
>> lower_clip_distance_visitor::visit_new_assignment(ir_assignment *ir)
>> >
>> >
>> > /**
>> > - * If gl_ClipDistance appears as an argument in an ir_call expression,
>> replace
>> > - * it with a temporary variable, and make sure the ir_call is preceded
>> and/or
>> > - * followed by assignments that copy the contents of the temporary
>> variable to
>> > - * and/or from gl_ClipDistance. Each of these assignments is then
>> lowered to
>> > - * refer to gl_ClipDistanceMESA.
>> > + * If a 1D gl_ClipDistance variable appears as an argument in an
>> ir_call
>> > + * expression, replace it with a temporary variable, and make sure the
>> ir_call
>> > + * is preceded and/or followed by assignments that copy the contents
>> of the
>> > + * temporary variable to and/or from gl_ClipDistance. Each of these
>> > + * assignments is then lowered to refer to gl_ClipDistanceMESA.
>> > + *
>> > + * We need to do a similar replacement for 2D gl_ClipDistance, however
>> since
>> > + * it's an input, the only case we need to address is where a 1D slice
>> of it
>> > + * is passed as an "in" parameter to an ir_call, e.g.:
>> > + *
>> > + * foo(gl_in[i].gl_ClipDistance)
>> > */
>> > ir_visitor_status
>> > lower_clip_distance_visitor::visit_leave(ir_call *ir)
>> > @@ -349,12 +454,12 @@ lower_clip_distance_visitor::visit_leave(ir_call
>> *ir)
>> > formal_param_node = formal_param_node->next;
>> > actual_param_node = actual_param_node->next;
>> >
>> > - ir_dereference_variable *deref =
>> actual_param->as_dereference_variable();
>> > - if (deref && deref->var == this->old_clip_distance_var) {
>> > - /* User is trying to pass the whole gl_ClipDistance array to a
>> > - * function call. Since we are reshaping gl_ClipDistance
>> from an
>> > - * array of floats to an array of vec4's, this isn't going to
>> work
>> > - * anymore, so use a temporary array instead.
>> > + if (this->is_clip_distance_vec8(actual_param)) {
>> > + /* User is trying to pass the whole 1D gl_ClipDistance array
>> (or a 1D
>> > + * slice of a 2D gl_ClipDistance array) to a function call.
>> Since we
>> > + * are reshaping gl_ClipDistance from an array of floats to
>> an array
>> > + * of vec4's, this isn't going to work anymore, so use a
>> temporary
>> > + * array instead.
>> > */
>> > ir_variable *temp_clip_distance = new(ctx) ir_variable(
>> > actual_param->type, "temp_clip_distance",
>> ir_var_temporary);
>> > @@ -370,7 +475,7 @@ lower_clip_distance_visitor::visit_leave(ir_call
>> *ir)
>> > */
>> > ir_assignment *new_assignment = new(ctx) ir_assignment(
>> > new(ctx) ir_dereference_variable(temp_clip_distance),
>> > - new(ctx)
>> ir_dereference_variable(old_clip_distance_var));
>> > + actual_param->clone(ctx, NULL));
>> > this->base_ir->insert_before(new_assignment);
>> > this->visit_new_assignment(new_assignment);
>> > }
>> > @@ -382,7 +487,7 @@ lower_clip_distance_visitor::visit_leave(ir_call
>> *ir)
>> > * afterwards to make sure it gets lowered.
>> > */
>> > ir_assignment *new_assignment = new(ctx) ir_assignment(
>> > - new(ctx) ir_dereference_variable(old_clip_distance_var),
>> > + actual_param->clone(ctx, NULL),
>> > new(ctx) ir_dereference_variable(temp_clip_distance));
>> > this->base_ir->insert_after(new_assignment);
>> > this->visit_new_assignment(new_assignment);
>> > @@ -401,8 +506,10 @@ lower_clip_distance(gl_shader *shader)
>> >
>> > visit_list_elements(&v, shader->ir);
>> >
>> > - if (v.new_clip_distance_var)
>> > - shader->symbols->add_variable(v.new_clip_distance_var);
>> > + if (v.new_clip_distance_1d_var)
>> > + shader->symbols->add_variable(v.new_clip_distance_1d_var);
>> > + if (v.new_clip_distance_2d_var)
>> > + shader->symbols->add_variable(v.new_clip_distance_2d_var);
>> >
>> > return v.progress;
>> > }
>> >
>>
>>
>
Any further thoughts about this, Ian?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131005/4ed9fc0c/attachment-0001.html>
More information about the mesa-dev
mailing list