[Mesa-dev] [PATCH 1/9] glsl: Consolidate ir_expression constructors that use explicit types.

Eric Anholt eric at anholt.net
Wed Feb 20 12:25:09 PST 2013


Matt Turner <mattst88 at gmail.com> writes:

> From: Kenneth Graunke <kenneth at whitecape.org>
>
> Previously, we had separate constructors for one, two, and four operand
> expressions.  This patch consolidates them into a single constructor
> which uses NULL default parameters.
>
> The unary and binary operator constructors had assertions to verify that
> the caller supplied the correct number of operands for the expression,
> but the four-operand version did not.  Since get_num_operands for
> ir_quadop_vector returns the number of vector_elements, we can safely
> add that without breaking the semantics of ir_quadop_vector.
>
> This also paves the way for expressions with three operands.  Currently,
> none can be constructed since get_num_operands() never returns 3.
>
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
> ---
>  src/glsl/ir.cpp |   34 ++++++----------------------------
>  src/glsl/ir.h   |   13 ++++---------
>  2 files changed, 10 insertions(+), 37 deletions(-)
>
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index 954995d..4ccdc42 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -234,6 +206,12 @@ ir_expression::ir_expression(int op, const struct glsl_type *type,
>     this->operands[1] = op1;
>     this->operands[2] = op2;
>     this->operands[3] = op3;
> +#ifndef NDEBUG
> +   int num_operands = get_num_operands(this->operation);
> +   for (int i = num_operands; i < 4; i++) {
> +      assert(this->operands[i] == NULL);
> +   }
> +#endif

Ugh, I don't like seeing #ifdefs for debug around when it's avoidable --
we should be able to rely on this stuff getting optimized out.  Except
that the optimizer doesn't get to know that get_num_operands() has no
side effects, unless we annotate it with __attribute__((pure)).  I guess
I'm fine either way -- this, or adding the attribute.

So, other than fixing the added tabs in new lines of some patches
(particularly #6), patches 1 and 3-9 are

Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130220/9083f15c/attachment.pgp>


More information about the mesa-dev mailing list