On 31 October 2011 23:24, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On 10/31/2011 08:34 PM, Paul Berry wrote:
> > When converting an expression like "++x" to GLSL IR we were failing to
> > account for the possibility that x might be an unsigned integral type.
> > As a result the user would receive a bogus error message "Could not
> > implicitly convert operands to arithmetic operator".
> >
> > Fixes piglit tests {vs,fs}-{increment,decrement}-uint.
> > ---
> > src/glsl/ast_to_hir.cpp | 31 +++++++++++++++++++++++--------
> > 1 files changed, 23 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> > index 7584fdf..a4eec50 100644
> > --- a/src/glsl/ast_to_hir.cpp
> > +++ b/src/glsl/ast_to_hir.cpp
> > @@ -935,6 +935,27 @@ check_builtin_array_max_size(const char *name,
> unsigned size,
> > return false;
> > }
> >
> > +/**
> > + * Create the constant 1, with the given GLSL type, for use in
> increment and
> > + * decrement operators.
> > + *
> > + * If the given type is invalid for increment and decrement operators,
> return
> > + * a floating point 1--the error will be detected later.
> > + */
> > +static ir_rvalue *
> > +constant_one_for_inc_dec(void *ctx, const glsl_type *type)
> > +{
> > + switch (type->base_type) {
> > + case GLSL_TYPE_UINT:
> > + return new(ctx) ir_constant((unsigned) 1);
> > + case GLSL_TYPE_INT:
> > + return new(ctx) ir_constant(1);
> > + default:
> > + case GLSL_TYPE_FLOAT:
> > + return new(ctx) ir_constant(1.0f);
> > + }
> > +}
>
> Perhaps it would be cleaner to add a new ir_constant::one static method
> similar to ir_constant::zero? There's nothing really inc/dec specific
> about this, and it might be more useful in general...
>
The difficulty is that this function doesn't actually return a constant 1
of the given type--it returns a constant 1 of a type which is suitable for
incrementing and decrementing values of the given type. For example, if
type is vec4, this function creates a constant value of 1.0 having type
float. (My comments in the code seem to contradict this--I'll fix that).
IMHO, that makes this function specific enough to increment/decrement that
it belongs here.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
