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

[Eric Anholt]

Paul Berry <stereotype441 at gmail.com> writes:

> On 20 February 2013 12:25, Eric Anholt <eric at anholt.net> wrote:
>
>> 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.
>>
>
> In i965 I would be a fan of adding the attribute, but since this code is in
> glsl, it needs to be buildable using MSVC, which does not support
> __attribute__ declarations.

We #ifdef these things based on __GNUC__, so that lame compilers keep
compiling but suffer.
-------------- 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/c98a1317/attachment.pgp>