[Mesa-dev] [PATCH 2/2] glsl: Implement a lowering pass for gl_ClipDistance.
Kenneth Graunke
kenneth at whitecape.org
Wed Sep 21 18:21:46 PDT 2011
On 09/15/2011 04:40 PM, Paul Berry wrote:
> In i965 GEN6+ (and I suspect most other hardware), gl_ClipDistance
> needs to be laid out as a pair of vec4's (the first containing clip
> distances 0-3, and the second containing clip distances 4-7).
> However, it is declared in GLSL as an array of 8 floats.
>
> This lowering pass acts at the GLSL level, modifying the declaration
> of gl_ClipDistance so that it is an array of vec4's rather than an
> array of floats, and renaming it to gl_ClipDistanceMESA. In addition,
> it modifies all accesses to the array so that they access the
> appropiate component of one of the vec4's.
>
> Since some hardware may not internally represent gl_ClipDistance as a
> pair of vec4's, this lowering pass is optional. To enable it, set the
> LowerClipDistance flag in gl_shader_compiler_options to true.
> ---
> src/glsl/Makefile | 1 +
> src/glsl/SConscript | 1 +
> src/glsl/ir_optimization.h | 1 +
> src/glsl/linker.cpp | 3 +
> src/glsl/lower_clip_distance.cpp | 347 ++++++++++++++++++++++++++++++++++++++
> src/mesa/main/mtypes.h | 1 +
> 6 files changed, 354 insertions(+), 0 deletions(-)
> create mode 100644 src/glsl/lower_clip_distance.cpp
>
> diff --git a/src/glsl/Makefile b/src/glsl/Makefile
> index c20a6c9..bee21c2 100644
> --- a/src/glsl/Makefile
> +++ b/src/glsl/Makefile
> @@ -56,6 +56,7 @@ CXX_SOURCES = \
> loop_analysis.cpp \
> loop_controls.cpp \
> loop_unroll.cpp \
> + lower_clip_distance.cpp \
> lower_discard.cpp \
> lower_if_to_cond_assign.cpp \
> lower_instructions.cpp \
> diff --git a/src/glsl/SConscript b/src/glsl/SConscript
> index 1da58a9..b4786c5 100644
> --- a/src/glsl/SConscript
> +++ b/src/glsl/SConscript
> @@ -67,6 +67,7 @@ glsl_sources = [
> 'loop_analysis.cpp',
> 'loop_controls.cpp',
> 'loop_unroll.cpp',
> + 'lower_clip_distance.cpp',
> 'lower_discard.cpp',
> 'lower_if_to_cond_assign.cpp',
> 'lower_instructions.cpp',
> diff --git a/src/glsl/ir_optimization.h b/src/glsl/ir_optimization.h
> index 48448d4..af80e26 100644
> --- a/src/glsl/ir_optimization.h
> +++ b/src/glsl/ir_optimization.h
> @@ -69,6 +69,7 @@ bool lower_noise(exec_list *instructions);
> bool lower_variable_index_to_cond_assign(exec_list *instructions,
> bool lower_input, bool lower_output, bool lower_temp, bool lower_uniform);
> bool lower_quadop_vector(exec_list *instructions, bool dont_lower_swz);
> +bool lower_clip_distance(exec_list *instructions);
> bool optimize_redundant_jumps(exec_list *instructions);
>
> ir_rvalue *
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 195f58f..ceb49fd 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1743,6 +1743,9 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
> if (!prog->LinkStatus)
> goto done;
>
> + if (ctx->ShaderCompilerOptions[i].LowerClipDistance)
> + lower_clip_distance(prog->_LinkedShaders[i]->ir);
> +
> while (do_common_optimization(prog->_LinkedShaders[i]->ir, true, 32))
> ;
> }
> diff --git a/src/glsl/lower_clip_distance.cpp b/src/glsl/lower_clip_distance.cpp
> new file mode 100644
> index 0000000..55a9e9d
> --- /dev/null
> +++ b/src/glsl/lower_clip_distance.cpp
> @@ -0,0 +1,347 @@
> +/*
> + * Copyright © 2011 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * \file lower_clip_distance.cpp
> + *
> + * This pass accounts for the difference between the way
> + * gl_ClipDistance is declared in standard GLSL (as an array of
> + * floats), and the way it is frequently implemented in hardware (as
> + * a pair of vec4s, with four clip distances packed into each).
> + *
> + * The declaration of gl_ClipDistance is replaced with a declaration
> + * of gl_ClipDistanceMESA, and any references to gl_ClipDistance are
> + * translated to refer to gl_ClipDistanceMESA with the appropriate
> + * swizzling of array indices. For instance:
> + *
> + * gl_ClipDistance[i]
> + *
> + * is translated into:
> + *
> + * gl_ClipDistanceMESA[i>>2][i&3]
> + *
> + * Since some hardware may not internally represent gl_ClipDistance as a pair
> + * of vec4's, this lowering pass is optional. To enable it, set the
> + * LowerClipDistance flag in gl_shader_compiler_options to true.
> + */
> +
> +#include "ir_hierarchical_visitor.h"
> +#include "ir.h"
> +
> +class lower_clip_distance_visitor : public ir_hierarchical_visitor {
> +public:
> + lower_clip_distance_visitor()
> + : progress(false), old_clip_distance_var(NULL),
> + new_clip_distance_var(NULL)
> + {
> + }
> +
> + virtual ir_visitor_status visit(ir_variable *);
> + void create_indices(ir_rvalue*, ir_rvalue *&, ir_rvalue *&);
> + virtual ir_visitor_status visit_leave(ir_dereference_array *);
> + virtual ir_visitor_status visit_leave(ir_assignment *);
> + void visit_new_assignment(ir_assignment *ir);
> + virtual ir_visitor_status visit_leave(ir_call *);
> +
> + bool progress;
> +
> + /**
> + * Pointer to the declaration of gl_ClipDistance, if found.
> + */
> + ir_variable *old_clip_distance_var;
> +
> + /**
> + * Pointer to the newly-created gl_ClipDistanceMESA variable.
> + */
> + ir_variable *new_clip_distance_var;
> +};
> +
> +
> +/**
> + * Replace any declaration of gl_ClipDistance as an array of floats with a
> + * declaration of gl_ClipDistanceMESA as an array of vec4's.
> + */
> +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)
> + 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());
> + assert (ir->type->element_type() == glsl_type::float_type);
assert(...) please.
> + 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);
> +
> + /* 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
> + = glsl_type::get_array_instance(glsl_type::vec4_type, new_size);
> + this->new_clip_distance_var->max_array_access = ir->max_array_access / 4;
> +
> + ir->replace_with(this->new_clip_distance_var);
> + }
> + return visit_continue;
> +}
> +
> +
> +/**
> + * Create the necessary GLSL rvalues to index into gl_ClipDistanceMESA based
> + * on the rvalue previously used to index into gl_ClipDistance. The first,
> + * "outer" index, selects one of the vec4's in gl_ClipDistanceMESA, and the
> + * second, "inner" index, selects a component within that vec4.
> + */
> +void
> +lower_clip_distance_visitor::create_indices(ir_rvalue *old_index,
> + ir_rvalue *&outer_index,
> + ir_rvalue *&inner_index)
> +{
> + void *ctx = ralloc_parent(old_index);
> +
> + /* Make sure old_index is a signed int so that the bitwise "shift" and
> + * "and" operations below type check properly.
> + */
> + if (old_index->type != glsl_type::int_type) {
> + assert (old_index->type == glsl_type::uint_type);
> + old_index = new(ctx) ir_expression(ir_unop_u2i, old_index);
> + }
FWIW, I'm completely okay with your handling of uints.
> + ir_constant *old_index_constant = old_index->constant_expression_value();
> + if (old_index_constant) {
> + /* gl_ClipDistance is being accessed via a constant index. Don't bother
> + * creating expressions to calculate the lowered indices. Just create
> + * constants.
> + */
> + int const_val = old_index_constant->get_int_component(0);
> + outer_index = new(ctx) ir_constant(const_val / 4);
> + inner_index = new(ctx) ir_constant(const_val % 4);
> + } else {
> + /* Create a variable to hold the value of old_index (so that we
> + * don't compute it twice).
> + */
> + ir_variable *old_index_var = new(ctx) ir_variable(
> + glsl_type::int_type, "clip_distance_index", ir_var_temporary);
> + this->base_ir->insert_before(old_index_var);
> + this->base_ir->insert_before(new(ctx) ir_assignment(
> + new(ctx) ir_dereference_variable(old_index_var), old_index));
> +
> + /* Create the expression clip_distance_index / 4. Do this as a bit
> + * shift because that's likely to be more efficient.
> + */
> + outer_index = new(ctx) ir_expression(
> + ir_binop_rshift, new(ctx) ir_dereference_variable(old_index_var),
> + new(ctx) ir_constant(2));
> +
> + /* Create the expression clip_distance_index % 4. Do this as a bitwise
> + * AND because that's likely to be more efficient.
> + */
> + inner_index = new(ctx) ir_expression(
> + ir_binop_bit_and, new(ctx) ir_dereference_variable(old_index_var),
> + new(ctx) ir_constant(3));
> + }
> +}
> +
> +
> +/**
> + * Replace any expression that indexes into the gl_ClipDistance array with an
> + * expression that indexes into one of the vec4's in gl_ClipDistanceMESA and
> + * accesses the appropriate component.
> + */
> +ir_visitor_status
> +lower_clip_distance_visitor::visit_leave(ir_dereference_array *ir)
> +{
> + /* 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)
> + return visit_continue;
> +
> + ir_dereference_variable *old_var_ref = ir->array->as_dereference_variable();
> + if (old_var_ref && old_var_ref->variable_referenced()
> + == this->old_clip_distance_var) {
> + this->progress = true;
> + ir_rvalue *outer_index;
> + ir_rvalue *inner_index;
> + this->create_indices(ir->array_index, outer_index, inner_index);
> + void *mem_ctx = ralloc_parent(ir);
> + ir->array = new(mem_ctx) ir_dereference_array(
> + this->new_clip_distance_var, outer_index);
> + ir->array_index = inner_index;
> + }
> +
> + return visit_continue;
> +}
> +
> +
> +/**
> + * 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.
> + */
> +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->variable_referenced()
> + == this->old_clip_distance_var)
> + || (rhs_var && rhs_var->variable_referenced()
> + == this->old_clip_distance_var)) {
If you want, you can just do lhs_var->var and rhs_var->var rather than
using the variable_referenced() function. It might make things fit on
one line and the indentation a little easier to follow. I don't have a
strong preference.
> + /* 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.
> + *
> + * Note: to unroll into element-by-element assignments, we need to make
> + * clones of the LHS and RHS. This is only safe if the LHS and RHS are
> + * side-effect free. Fortunately, we know that they are, because the
> + * only kind of rvalue that can have side effects is an ir_call, and
> + * ir_calls only appear (a) as a statement on their own, or (b) as the
> + * RHS of an assignment that stores the result of the call in a
> + * temporary variable.
> + */
> + void *ctx = ralloc_parent(ir);
> + int array_size = this->old_clip_distance_var->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));
> + this->visit_leave(new_lhs);
I think you want:
new_lhs->accept(this);
> + ir_dereference_array *new_rhs = new(ctx) ir_dereference_array(
> + ir->rhs->clone(ctx, NULL), new(ctx) ir_constant(i));
> + this->visit_leave(new_rhs);
Ditto:
new_rhs->accept(this);
> + this->base_ir->insert_before(
> + new(ctx) ir_assignment(new_lhs, new_rhs));
> + }
> + ir->remove();
> + }
> +
> + return visit_continue;
> +}
> +
> +
> +/**
> + * Set up base_ir properly and call visit_leave() on a newly created
> + * ir_assignment node. This is used in cases where we have to insert an
> + * ir_assignment in a place where we know the hierarchical visitor won't see
> + * it.
> + */
> +void
> +lower_clip_distance_visitor::visit_new_assignment(ir_assignment *ir)
> +{
> + ir_instruction *old_base_ir = this->base_ir;
> + this->base_ir = ir;
Blargh. Annoying that you have to do this. But seems right.
> + this->visit_leave(ir);
Again:
this->accept(ir);
> + this->base_ir = old_base_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.
> + */
> +ir_visitor_status
> +lower_clip_distance_visitor::visit_leave(ir_call *ir)
> +{
> + void *ctx = ralloc_parent(ir);
> +
> + const exec_node *formal_param_node = ir->get_callee()->parameters.head;
> + const exec_node *actual_param_node = ir->actual_parameters.head;
> + while (!actual_param_node->is_tail_sentinel()) {
> + ir_variable *formal_param = (ir_variable *) formal_param_node;
> + ir_rvalue *actual_param = (ir_rvalue *) actual_param_node;
> +
> + /* Advance formal_param_node and actual_param_node now so that we can
> + * safely replace actual_param with another node, if necessary, below.
> + */
> + 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->variable_referenced()
> + == this->old_clip_distance_var) {
> + /* User is trying to pass gl_ClipDistance as an in, out, or inout
Perhaps "User is trying to pass the whole gl_ClipDistance array to a
function call." This would clarify that it's not gl_ClipDistance[3] or
such. Plus "in, out, or inout" is just enumerating all the options. :)
> + * parameter 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);
> + this->base_ir->insert_before(temp_clip_distance);
> + actual_param->replace_with(
> + new(ctx) ir_dereference_variable(temp_clip_distance));
> + if (formal_param->mode == ir_var_in
> + || formal_param->mode == ir_var_inout) {
> + /* Copy from gl_ClipDistance to the temporary before the call.
> + * Since we are going to insert this copy before the current
> + * instruction, we need to visit it afterwards to make sure it
> + * gets lowered.
> + */
> + ir_assignment *new_assignment = new(ctx) ir_assignment(
> + new(ctx) ir_dereference_variable(temp_clip_distance),
> + actual_param->clone(ctx, NULL));
I had to reread this several times before I realized that actual_param,
in the list, has been replaced with a new dereference of
temp_clip_distance, but the /local pointer/ actual_param is still the
old value being passed in...namely (var_ref old_clip_distance_var). I
thought you were doing temp_clip_distance = temp_clip_distance at first.
Perhaps this would be clearer:
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));
> + this->base_ir->insert_before(new_assignment);
> + this->visit_new_assignment(new_assignment);
> + }
> + if (formal_param->mode == ir_var_out
> + || formal_param->mode == ir_var_inout) {
> + /* Copy from the temporary to gl_ClipDistance after the call.
> + * Since visit_list_elements() has already decided which
> + * instruction it's going to visit next, we need to visit
> + * afterwards to make sure it gets lowered.
> + */
> + ir_assignment *new_assignment = new(ctx) ir_assignment(
> + actual_param->clone(ctx, NULL),
Ditto...
> + new(ctx) ir_dereference_variable(temp_clip_distance));
> + this->base_ir->insert_after(new_assignment);
> + this->visit_new_assignment(new_assignment);
> + }
> + }
> + }
> +
> + return visit_continue;
> +}
> +
> +
> +bool
> +lower_clip_distance(exec_list *instructions)
> +{
> + lower_clip_distance_visitor v;
> +
> + visit_list_elements(&v, instructions);
> +
> + return v.progress;
> +}
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 8671ecd..ef04749 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2297,6 +2297,7 @@ struct gl_shader_compiler_options
> GLboolean EmitNoMainReturn; /**< Emit CONT/RET opcodes? */
> GLboolean EmitNoNoise; /**< Emit NOISE opcodes? */
> GLboolean EmitNoPow; /**< Emit POW opcodes? */
> + GLboolean LowerClipDistance; /**< Lower gl_ClipDistance from float[8] to vec4[2]? */
>
> /**
> * \name Forms of indirect addressing the driver cannot do.
Overall, this looks great, Paul. Other than my few small comments:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
More information about the mesa-dev
mailing list