[Mesa-dev] [PATCH v2 16/28] glsl/ast: Support double floats
Ian Romanick
idr at freedesktop.org
Fri Feb 6 00:32:06 PST 2015
Three comments near the middle / bottom...
On 02/06/2015 06:56 AM, Ilia Mirkin wrote:
> From: Dave Airlie <airlied at gmail.com>
>
> Signed-off-by: Dave Airlie <airlied at redhat.com>
> ---
> src/glsl/ast.h | 2 ++
> src/glsl/ast_function.cpp | 67 +++++++++++++++++++++++++++++++++--------
> src/glsl/ast_to_hir.cpp | 38 +++++++++++++++++++++--
> src/glsl/glsl_parser_extras.cpp | 4 +++
> 4 files changed, 96 insertions(+), 15 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 6995ae8..ef74e51 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -189,6 +189,7 @@ enum ast_operators {
> ast_uint_constant,
> ast_float_constant,
> ast_bool_constant,
> + ast_double_constant,
>
> ast_sequence,
> ast_aggregate
> @@ -236,6 +237,7 @@ public:
> float float_constant;
> unsigned uint_constant;
> int bool_constant;
> + double double_constant;
> } primary_expression;
>
>
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index cbff9d8..5401390 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -573,6 +573,9 @@ convert_component(ir_rvalue *src, const glsl_type *desired_type)
> result = new(ctx) ir_expression(ir_unop_i2u,
> new(ctx) ir_expression(ir_unop_b2i, src));
> break;
> + case GLSL_TYPE_DOUBLE:
> + result = new(ctx) ir_expression(ir_unop_d2u, src);
> + break;
> }
> break;
> case GLSL_TYPE_INT:
> @@ -586,6 +589,9 @@ convert_component(ir_rvalue *src, const glsl_type *desired_type)
> case GLSL_TYPE_BOOL:
> result = new(ctx) ir_expression(ir_unop_b2i, src);
> break;
> + case GLSL_TYPE_DOUBLE:
> + result = new(ctx) ir_expression(ir_unop_d2i, src);
> + break;
> }
> break;
> case GLSL_TYPE_FLOAT:
> @@ -599,6 +605,9 @@ convert_component(ir_rvalue *src, const glsl_type *desired_type)
> case GLSL_TYPE_BOOL:
> result = new(ctx) ir_expression(ir_unop_b2f, desired_type, src, NULL);
> break;
> + case GLSL_TYPE_DOUBLE:
> + result = new(ctx) ir_expression(ir_unop_d2f, desired_type, src, NULL);
> + break;
> }
> break;
> case GLSL_TYPE_BOOL:
> @@ -613,8 +622,28 @@ convert_component(ir_rvalue *src, const glsl_type *desired_type)
> case GLSL_TYPE_FLOAT:
> result = new(ctx) ir_expression(ir_unop_f2b, desired_type, src, NULL);
> break;
> + case GLSL_TYPE_DOUBLE:
> + result = new(ctx) ir_expression(ir_unop_f2b,
> + new(ctx) ir_expression(ir_unop_d2f, src));
> + break;
> }
> break;
> + case GLSL_TYPE_DOUBLE:
> + switch (b) {
> + case GLSL_TYPE_INT:
> + result = new(ctx) ir_expression(ir_unop_i2d, src);
> + break;
> + case GLSL_TYPE_UINT:
> + result = new(ctx) ir_expression(ir_unop_u2d, src);
> + break;
> + case GLSL_TYPE_BOOL:
> + result = new(ctx) ir_expression(ir_unop_f2d,
> + new(ctx) ir_expression(ir_unop_b2f, src));
> + break;
> + case GLSL_TYPE_FLOAT:
> + result = new(ctx) ir_expression(ir_unop_f2d, desired_type, src, NULL);
> + break;
> + }
> }
>
> assert(result != NULL);
> @@ -711,9 +740,9 @@ process_vec_mat_constructor(exec_list *instructions,
>
> /* Apply implicit conversions (not the scalar constructor rules!). See
> * the spec quote above. */
> - if (constructor_type->is_float()) {
> + if (constructor_type->base_type != result->type->base_type) {
> const glsl_type *desired_type =
> - glsl_type::get_instance(GLSL_TYPE_FLOAT,
> + glsl_type::get_instance(constructor_type->base_type,
> ir->type->vector_elements,
> ir->type->matrix_columns);
> if (result->type->can_implicitly_convert_to(desired_type, state)) {
> @@ -847,13 +876,17 @@ process_array_constructor(exec_list *instructions,
> foreach_in_list_safe(ir_rvalue, ir, &actual_parameters) {
> ir_rvalue *result = ir;
>
> + const glsl_base_type element_base_type =
> + constructor_type->element_type()->base_type;
> +
> /* Apply implicit conversions (not the scalar constructor rules!). See
> * the spec quote above. */
> - if (constructor_type->element_type()->is_float()) {
> - const glsl_type *desired_type =
> - glsl_type::get_instance(GLSL_TYPE_FLOAT,
> - ir->type->vector_elements,
> - ir->type->matrix_columns);
> + if (element_base_type != result->type->base_type) {
> + const glsl_type *desired_type =
> + glsl_type::get_instance(element_base_type,
> + ir->type->vector_elements,
> + ir->type->matrix_columns);
> +
It's not obvious to me why this hunk is correct. It seems like it does
more than add support for doubles... but I'm having a little trouble
grokking it (I'm coming down with a cold, so that isn't helping).
> if (result->type->can_implicitly_convert_to(desired_type, state)) {
> /* Even though convert_component() implements the constructor
> * conversion rules (not the implicit conversion rules), its safe
> @@ -1012,6 +1045,9 @@ emit_inline_vector_constructor(const glsl_type *type,
> case GLSL_TYPE_FLOAT:
> data.f[i + base_component] = c->get_float_component(i);
> break;
> + case GLSL_TYPE_DOUBLE:
> + data.d[i + base_component] = c->get_double_component(i);
> + break;
> case GLSL_TYPE_BOOL:
> data.b[i + base_component] = c->get_bool_component(i);
> break;
> @@ -1167,16 +1203,21 @@ emit_inline_matrix_constructor(const glsl_type *type,
> /* Assign the scalar to the X component of a vec4, and fill the remaining
> * components with zero.
> */
> + glsl_base_type param_base_type = first_param->type->base_type;
> + assert(param_base_type == GLSL_TYPE_FLOAT ||
> + param_base_type == GLSL_TYPE_DOUBLE);
> ir_variable *rhs_var =
> - new(ctx) ir_variable(glsl_type::vec4_type, "mat_ctor_vec",
> - ir_var_temporary);
> + new(ctx) ir_variable(glsl_type::get_instance(param_base_type, 4, 1),
> + "mat_ctor_vec",
> + ir_var_temporary);
> instructions->push_tail(rhs_var);
>
> ir_constant_data zero;
> - zero.f[0] = 0.0;
> - zero.f[1] = 0.0;
> - zero.f[2] = 0.0;
> - zero.f[3] = 0.0;
> + for (unsigned i = 0; i < 4; i++)
> + if (param_base_type == GLSL_TYPE_FLOAT)
> + zero.f[i] = 0.0;
> + else
> + zero.d[i] = 0.0;
>
> ir_instruction *inst =
> new(ctx) ir_assignment(new(ctx) ir_dereference_variable(rhs_var),
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index ed0eb09..3ea3a13 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -172,6 +172,7 @@ get_conversion_operation(const glsl_type *to, const glsl_type *from,
> switch (from->base_type) {
> case GLSL_TYPE_INT: return ir_unop_i2f;
> case GLSL_TYPE_UINT: return ir_unop_u2f;
> + case GLSL_TYPE_DOUBLE: return ir_unop_d2f;
> default: return (ir_expression_operation)0;
> }
>
> @@ -183,6 +184,16 @@ get_conversion_operation(const glsl_type *to, const glsl_type *from,
> default: return (ir_expression_operation)0;
> }
>
> + case GLSL_TYPE_DOUBLE:
> + if (!state->has_double())
> + return (ir_expression_operation)0;
> + switch (from->base_type) {
> + case GLSL_TYPE_INT: return ir_unop_i2d;
> + case GLSL_TYPE_UINT: return ir_unop_u2d;
> + case GLSL_TYPE_FLOAT: return ir_unop_f2d;
> + default: return (ir_expression_operation)0;
> + }
> +
> default: return (ir_expression_operation)0;
> }
> }
> @@ -340,8 +351,10 @@ arithmetic_result_type(ir_rvalue * &value_a, ir_rvalue * &value_b,
> * type of both operands must be float.
> */
> assert(type_a->is_matrix() || type_b->is_matrix());
> - assert(type_a->base_type == GLSL_TYPE_FLOAT);
> - assert(type_b->base_type == GLSL_TYPE_FLOAT);
> + assert(type_a->base_type == GLSL_TYPE_FLOAT ||
> + type_a->base_type == GLSL_TYPE_DOUBLE);
> + assert(type_b->base_type == GLSL_TYPE_FLOAT ||
> + type_b->base_type == GLSL_TYPE_DOUBLE);
>
> /* "* The operator is add (+), subtract (-), or divide (/), and the
> * operands are matrices with the same number of rows and the same
> @@ -959,6 +972,7 @@ do_comparison(void *mem_ctx, int operation, ir_rvalue *op0, ir_rvalue *op1)
> case GLSL_TYPE_UINT:
> case GLSL_TYPE_INT:
> case GLSL_TYPE_BOOL:
> + case GLSL_TYPE_DOUBLE:
> return new(mem_ctx) ir_expression(operation, op0, op1);
>
> case GLSL_TYPE_ARRAY: {
> @@ -1746,6 +1760,10 @@ ast_expression::do_hir(exec_list *instructions,
> result = new(ctx) ir_constant(bool(this->primary_expression.bool_constant));
> break;
>
> + case ast_double_constant:
> + result = new(ctx) ir_constant(this->primary_expression.double_constant);
> + break;
> +
> case ast_sequence: {
> /* It should not be possible to generate a sequence in the AST without
> * any expressions in it.
> @@ -2560,6 +2578,12 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
> _mesa_glsl_error(loc, state,
> "varying variables may not be of type struct");
> break;
> + case GLSL_TYPE_DOUBLE:
> + if (state->has_double())
> + break;
> + _mesa_glsl_error(loc, state,
> + "varying variables may not be of type double");
> + break;
This block isn't necessary, is it? Doesn't the lexer generate an error
if you use double with when has_double() is false? I think both
GLSL_TYPE_DOUBLE and GLSL_TYPE_UINT (for the same reason) could be
handled like GLSL_TYPE_FLOAT.
> default:
> _mesa_glsl_error(loc, state, "illegal type for a varying variable");
> break;
> @@ -3645,6 +3669,16 @@ ast_declarator_list::hir(exec_list *instructions,
> var_type);
> }
>
> + /* Double fragment inputs must be qualified with 'flat'. */
> + if (state->has_double() &&
Same here. The has_double() check is unnecessary. In the int case, a
version check is necessary because the int type existed in GLSL 1.20,
but you could not use it for some things.
> + var->type->contains_double() &&
> + var->data.interpolation != INTERP_QUALIFIER_FLAT &&
> + state->stage == MESA_SHADER_FRAGMENT &&
> + var->data.mode == ir_var_shader_in) {
> + _mesa_glsl_error(&loc, state, "if a fragment input is (or contains) "
> + "a double, then it must be qualified with 'flat'",
> + var_type);
> + }
>
> /* Interpolation qualifiers cannot be applied to 'centroid' and
> * 'centroid varying'.
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index cb19ce1..29e6dd2 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -962,6 +962,10 @@ ast_expression::print(void) const
> printf("%f ", primary_expression.float_constant);
> break;
>
> + case ast_double_constant:
> + printf("%f ", primary_expression.double_constant);
> + break;
> +
> case ast_bool_constant:
> printf("%s ",
> primary_expression.bool_constant
>
More information about the mesa-dev
mailing list