[gtk+/wip/otte/shader: 264/269] gskslexpression: Make gsk_sl_expression_write_spv() take a type argument
- From: Benjamin Otte <otte src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gtk+/wip/otte/shader: 264/269] gskslexpression: Make gsk_sl_expression_write_spv() take a type argument
- Date: Tue, 24 Oct 2017 06:12:49 +0000 (UTC)
commit dc50bba2c1f020d817176782b0548d45c266cd29
Author: Benjamin Otte <otte redhat com>
Date: Mon Oct 23 23:43:06 2017 +0200
gskslexpression: Make gsk_sl_expression_write_spv() take a type argument
This is an optional argument that forces a conversion to this type. The
type must be compatible with the expression's return type.
This achieves 2 things:
1. It coerces constants to the correct type before they get written to
SPV, like when doing
float f = 1;
we used to save an integer constant in the SPV and then convert it to
float.
2. It makes people think about the return type when calling this
function.
3 cases (variable initialization and return types) did not properly
convert values before. This is not fixed.
gsk/gsksldeclaration.c | 4 +-
gsk/gskslexpression.c | 109 ++++++++++----------
gsk/gskslexpressionprivate.h | 3 +-
gsk/gskslstatement.c | 68 ++++++++-----
gsk/gskspvwriter.c | 2 +-
testsuite/gsksl/errors/invalid-return-type.glsl | 11 ++
testsuite/gsksl/errors/returning-no-value.glsl | 11 ++
.../gsksl/errors/returning-value-from-void.glsl | 11 ++
8 files changed, 134 insertions(+), 85 deletions(-)
---
diff --git a/gsk/gsksldeclaration.c b/gsk/gsksldeclaration.c
index e8ae53b..8c32be1 100644
--- a/gsk/gsksldeclaration.c
+++ b/gsk/gsksldeclaration.c
@@ -114,7 +114,9 @@ gsk_sl_declaration_variable_write_initializer_spv (const GskSlDeclaration *decla
{
gsk_sl_variable_store_spv (variable->variable,
writer,
- gsk_sl_expression_write_spv (variable->initial, writer));
+ gsk_sl_expression_write_spv (variable->initial,
+ writer,
+ gsk_sl_variable_get_type
(variable->variable)));
}
}
diff --git a/gsk/gskslexpression.c b/gsk/gskslexpression.c
index 6253537..bfaeaa7 100644
--- a/gsk/gskslexpression.c
+++ b/gsk/gskslexpression.c
@@ -162,7 +162,7 @@ gsk_sl_expression_assignment_write_spv (const GskSlExpression *expression,
g_assert (chain);
ltype = gsk_sl_expression_get_return_type (assignment->lvalue),
rtype = gsk_sl_expression_get_return_type (assignment->rvalue),
- rvalue = gsk_sl_expression_write_spv (assignment->rvalue, writer);
+ rvalue = gsk_sl_expression_write_spv (assignment->rvalue, writer, NULL);
if (assignment->binary)
{
@@ -354,17 +354,20 @@ gsk_sl_expression_binary_write_spv (const GskSlExpression *expression,
GskSpvWriter *writer)
{
const GskSlExpressionBinary *binary = (const GskSlExpressionBinary *) expression;
+ GskSlType *ltype, *rtype;
guint left_id, right_id;
- left_id = gsk_sl_expression_write_spv (binary->left, writer);
- right_id = gsk_sl_expression_write_spv (binary->right, writer);
+ ltype = gsk_sl_expression_get_return_type (binary->left);
+ rtype = gsk_sl_expression_get_return_type (binary->right);
+ left_id = gsk_sl_expression_write_spv (binary->left, writer, NULL);
+ right_id = gsk_sl_expression_write_spv (binary->right, writer, NULL);
return gsk_sl_binary_write_spv (binary->binary,
writer,
binary->type,
- gsk_sl_expression_get_return_type (binary->left),
+ ltype,
left_id,
- gsk_sl_expression_get_return_type (binary->right),
+ rtype,
right_id);
}
@@ -452,7 +455,7 @@ gsk_sl_expression_logical_and_write_spv (const GskSlExpression *expression,
const GskSlExpressionLogicalAnd *logical_and = (const GskSlExpressionLogicalAnd *) expression;
guint32 current_id, after_id, and_id, left_id, right_id, result_id;
- left_id = gsk_sl_expression_write_spv (logical_and->left, writer);
+ left_id = gsk_sl_expression_write_spv (logical_and->left, writer, NULL);
current_id = gsk_spv_writer_get_label_id (writer);
and_id = gsk_spv_writer_make_id (writer);
@@ -464,7 +467,7 @@ gsk_sl_expression_logical_and_write_spv (const GskSlExpression *expression,
gsk_spv_writer_start_code_block (writer, and_id, 0, 0);
gsk_spv_writer_label (writer, GSK_SPV_WRITER_SECTION_CODE, and_id);
- right_id = gsk_sl_expression_write_spv (logical_and->right, writer);
+ right_id = gsk_sl_expression_write_spv (logical_and->right, writer, NULL);
gsk_spv_writer_branch (writer, after_id);
gsk_spv_writer_start_code_block (writer, after_id, 0, 0);
@@ -565,7 +568,7 @@ gsk_sl_expression_logical_or_write_spv (const GskSlExpression *expression,
const GskSlExpressionLogicalOr *logical_or = (const GskSlExpressionLogicalOr *) expression;
guint32 current_id, condition_id, after_id, or_id, left_id, right_id, result_id;
- left_id = gsk_sl_expression_write_spv (logical_or->left, writer);
+ left_id = gsk_sl_expression_write_spv (logical_or->left, writer, NULL);
current_id = gsk_spv_writer_get_label_id (writer);
or_id = gsk_spv_writer_make_id (writer);
@@ -578,7 +581,7 @@ gsk_sl_expression_logical_or_write_spv (const GskSlExpression *expression,
gsk_spv_writer_start_code_block (writer, or_id, 0, 0);
gsk_spv_writer_label (writer, GSK_SPV_WRITER_SECTION_CODE, or_id);
- right_id = gsk_sl_expression_write_spv (logical_or->right, writer);
+ right_id = gsk_sl_expression_write_spv (logical_or->right, writer, NULL);
gsk_spv_writer_branch (writer, after_id);
gsk_spv_writer_start_code_block (writer, after_id, 0, 0);
@@ -697,7 +700,7 @@ gsk_sl_expression_conditional_write_spv (const GskSlExpression *expression,
guint32 true_id, false_id, after_id;
guint32 condition_id, left_id, right_id, result_id;
- condition_id = gsk_sl_expression_write_spv (conditional->condition, writer);
+ condition_id = gsk_sl_expression_write_spv (conditional->condition, writer, NULL);
true_id = gsk_spv_writer_make_id (writer);
false_id = gsk_spv_writer_make_id (writer);
@@ -708,20 +711,12 @@ gsk_sl_expression_conditional_write_spv (const GskSlExpression *expression,
gsk_spv_writer_start_code_block (writer, true_id, 0, 0);
gsk_spv_writer_label (writer, GSK_SPV_WRITER_SECTION_CODE, true_id);
- left_id = gsk_sl_expression_write_spv (conditional->left, writer);
- left_id = gsk_spv_writer_convert (writer,
- left_id,
- gsk_sl_expression_get_return_type (conditional->left),
- conditional->type);
+ left_id = gsk_sl_expression_write_spv (conditional->left, writer, conditional->type);
gsk_spv_writer_branch (writer, after_id);
gsk_spv_writer_start_code_block (writer, false_id, 0, 0);
gsk_spv_writer_label (writer, GSK_SPV_WRITER_SECTION_CODE, false_id);
- right_id = gsk_sl_expression_write_spv (conditional->right, writer);
- right_id = gsk_spv_writer_convert (writer,
- right_id,
- gsk_sl_expression_get_return_type (conditional->right),
- conditional->type);
+ right_id = gsk_sl_expression_write_spv (conditional->right, writer, conditional->type);
gsk_spv_writer_branch (writer, after_id);
gsk_spv_writer_start_code_block (writer, after_id, 0, 0);
@@ -1022,7 +1017,7 @@ gsk_sl_expression_constructor_write_spv (const GskSlExpression *expression,
if (constructor->n_arguments == 1 && gsk_sl_type_is_scalar (value_type))
{
- value_id = gsk_sl_expression_write_spv (constructor->arguments[0], writer);
+ value_id = gsk_sl_expression_write_spv (constructor->arguments[0], writer, NULL);
if (gsk_sl_type_is_scalar (type))
{
@@ -1086,15 +1081,10 @@ gsk_sl_expression_constructor_write_spv (const GskSlExpression *expression,
guint32 col_ids[4], ids[4], one_id, zero_id;
GskSlValue *value;
- value_id = gsk_sl_expression_write_spv (constructor->arguments[0], writer);
+ value_id = gsk_sl_expression_write_spv (constructor->arguments[0],
+ writer,
+ gsk_sl_type_get_matching (value_type,
gsk_sl_type_get_scalar_type (type)));
- if (gsk_sl_type_get_scalar_type (value_type) != gsk_sl_type_get_scalar_type (type))
- {
- GskSlType *new_value_type = gsk_sl_type_get_matching (value_type, gsk_sl_type_get_scalar_type
(type));
- value_id = gsk_spv_writer_convert (writer, value_id, value_type, new_value_type);
- value_type = new_value_type;
- }
-
value = gsk_sl_value_new (scalar_type);
zero_id = gsk_spv_writer_get_id_for_zero (writer, scalar_type);
one_id = gsk_spv_writer_get_id_for_one (writer, scalar_type);
@@ -1124,8 +1114,7 @@ gsk_sl_expression_constructor_write_spv (const GskSlExpression *expression,
else if (constructor->n_arguments == 1 && gsk_sl_type_is_vector (type) && gsk_sl_type_is_vector
(value_type)
&& gsk_sl_type_get_length (type) == gsk_sl_type_get_length (value_type))
{
- value_id = gsk_sl_expression_write_spv (constructor->arguments[0], writer);
- return gsk_spv_writer_convert (writer, value_id, value_type, type);
+ return gsk_sl_expression_write_spv (constructor->arguments[0], writer, type);
}
else
{
@@ -1139,16 +1128,9 @@ gsk_sl_expression_constructor_write_spv (const GskSlExpression *expression,
for (arg = 0; arg < constructor->n_arguments; arg++)
{
value_type = gsk_sl_expression_get_return_type (constructor->arguments[arg]);
- value_id = gsk_sl_expression_write_spv (constructor->arguments[arg], writer);
- if (gsk_sl_type_get_scalar_type (value_type) != scalar)
- {
- GskSlType *new_type = gsk_sl_type_get_matching (value_type, scalar);
- value_id = gsk_spv_writer_convert (writer,
- value_id,
- value_type,
- new_type);
- value_type = new_type;
- }
+ value_id = gsk_sl_expression_write_spv (constructor->arguments[arg],
+ writer,
+ gsk_sl_type_get_matching (value_type, scalar));
if (gsk_sl_type_is_scalar (value_type))
{
@@ -1329,11 +1311,9 @@ gsk_sl_expression_function_call_write_spv (const GskSlExpression *expression,
for (i = 0; i < function_call->n_arguments; i++)
{
- arguments[i] = gsk_sl_expression_write_spv (function_call->arguments[i], writer);
- arguments[i] = gsk_spv_writer_convert (writer,
- arguments[i],
- gsk_sl_expression_get_return_type (function_call->arguments[i]),
- gsk_sl_function_get_argument_type (function_call->function, i));
+ arguments[i] = gsk_sl_expression_write_spv (function_call->arguments[i],
+ writer,
+ gsk_sl_function_get_argument_type
(function_call->function, i));
}
return gsk_sl_function_write_call_spv (function_call->function, writer, arguments);
@@ -1434,8 +1414,8 @@ gsk_sl_expression_index_write_spv (const GskSlExpression *expression,
GskSlType *type;
type = gsk_sl_expression_get_return_type (index->expr);
- index_id = gsk_sl_expression_write_spv (index->index_expr, writer);
- value_id = gsk_sl_expression_write_spv (index->expr, writer);
+ index_id = gsk_sl_expression_write_spv (index->index_expr, writer, NULL);
+ value_id = gsk_sl_expression_write_spv (index->expr, writer, type);
if (gsk_sl_type_is_vector (type))
{
return gsk_spv_writer_vector_extract_dynamic (writer,
@@ -1574,7 +1554,7 @@ gsk_sl_expression_member_write_spv (const GskSlExpression *expression,
return gsk_spv_writer_composite_extract (writer,
gsk_sl_type_get_member_type (type, member->id),
- gsk_sl_expression_write_spv (member->expr, writer),
+ gsk_sl_expression_write_spv (member->expr, writer, NULL),
(guint32[1]) { member->id },
1);
}
@@ -1751,7 +1731,7 @@ gsk_sl_expression_swizzle_write_spv (const GskSlExpression *expression,
guint32 expr_id;
type = gsk_sl_expression_get_return_type (swizzle->expr);
- expr_id = gsk_sl_expression_write_spv (swizzle->expr, writer);
+ expr_id = gsk_sl_expression_write_spv (swizzle->expr, writer, NULL);
if (gsk_sl_type_is_scalar (type))
{
@@ -1921,13 +1901,13 @@ gsk_sl_expression_negation_write_spv (const GskSlExpression *expression,
case GSK_SL_UINT:
return gsk_spv_writer_s_negate (writer,
type,
- gsk_sl_expression_write_spv (negation->expr, writer));
+ gsk_sl_expression_write_spv (negation->expr, writer, NULL));
case GSK_SL_FLOAT:
case GSK_SL_DOUBLE:
return gsk_spv_writer_f_negate (writer,
type,
- gsk_sl_expression_write_spv (negation->expr, writer));
+ gsk_sl_expression_write_spv (negation->expr, writer, NULL));
case GSK_SL_VOID:
case GSK_SL_BOOL:
@@ -3553,7 +3533,8 @@ gsk_sl_expression_get_constant (const GskSlExpression *expression)
guint32
gsk_sl_expression_write_spv (const GskSlExpression *expression,
- GskSpvWriter *writer)
+ GskSpvWriter *writer,
+ GskSlType *result_type)
{
GskSpvAccessChain *chain;
GskSlValue *constant;
@@ -3562,6 +3543,12 @@ gsk_sl_expression_write_spv (const GskSlExpression *expression,
constant = gsk_sl_expression_get_constant (expression);
if (constant)
{
+ if (result_type != NULL && !gsk_sl_type_equal (gsk_sl_value_get_type (constant), result_type))
+ {
+ GskSlValue *tmp = gsk_sl_value_new_convert (constant, result_type);
+ gsk_sl_value_free (constant);
+ constant = tmp;
+ }
result_id = gsk_spv_writer_get_id_for_value (writer, constant);
gsk_sl_value_free (constant);
@@ -3573,8 +3560,20 @@ gsk_sl_expression_write_spv (const GskSlExpression *expression,
{
result_id = gsk_spv_access_chain_load (chain);
gsk_spv_access_chain_free (chain);
- return result_id;
+ }
+ else
+ {
+ result_id = expression->class->write_spv (expression, writer);
+ }
+
+ if (result_type)
+ {
+ GskSlType *expr_type;
+
+ expr_type = gsk_sl_expression_get_return_type (expression);
+ if (!gsk_sl_type_equal (expr_type, result_type))
+ result_id = gsk_spv_writer_convert (writer, result_id, expr_type, result_type);
}
- return expression->class->write_spv (expression, writer);
+ return result_id;
}
diff --git a/gsk/gskslexpressionprivate.h b/gsk/gskslexpressionprivate.h
index 15ab40a..9b9ddb0 100644
--- a/gsk/gskslexpressionprivate.h
+++ b/gsk/gskslexpressionprivate.h
@@ -51,7 +51,8 @@ GskSlType * gsk_sl_expression_get_return_type (const GskSlExpr
GskSlValue * gsk_sl_expression_get_constant (const GskSlExpression *expression);
guint32 gsk_sl_expression_write_spv (const GskSlExpression *expression,
- GskSpvWriter *writer);
+ GskSpvWriter *writer,
+ GskSlType *result_type);
G_END_DECLS
diff --git a/gsk/gskslstatement.c b/gsk/gskslstatement.c
index db3c0fb..488ceeb 100644
--- a/gsk/gskslstatement.c
+++ b/gsk/gskslstatement.c
@@ -249,7 +249,9 @@ gsk_sl_statement_declaration_write_spv (const GskSlStatement *statement,
{
gsk_sl_variable_store_spv (declaration->variable,
writer,
- gsk_sl_expression_write_spv (declaration->initial, writer));
+ gsk_sl_expression_write_spv (declaration->initial,
+ writer,
+ gsk_sl_variable_get_type
(declaration->variable)));
}
return FALSE;
@@ -269,6 +271,7 @@ typedef struct _GskSlStatementReturn GskSlStatementReturn;
struct _GskSlStatementReturn {
GskSlStatement parent;
+ GskSlType *return_type;
GskSlExpression *value;
};
@@ -279,6 +282,7 @@ gsk_sl_statement_return_free (GskSlStatement *statement)
if (return_statement->value)
gsk_sl_expression_unref (return_statement->value);
+ gsk_sl_type_unref (return_statement->return_type);
g_slice_free (GskSlStatementReturn, return_statement);
}
@@ -313,7 +317,9 @@ gsk_sl_statement_return_write_spv (const GskSlStatement *statement,
if (return_statement->value)
{
gsk_spv_writer_return_value (writer,
- gsk_sl_expression_write_spv (return_statement->value, writer));
+ gsk_sl_expression_write_spv (return_statement->value,
+ writer,
+ return_statement->return_type));
}
else
{
@@ -451,7 +457,7 @@ gsk_sl_statement_if_write_spv (const GskSlStatement *statement,
GskSlStatementIf *if_stmt = (GskSlStatementIf *) statement;
guint32 if_id, else_id, after_id, condition_id;
- condition_id = gsk_sl_expression_write_spv (if_stmt->condition, writer);
+ condition_id = gsk_sl_expression_write_spv (if_stmt->condition, writer, NULL);
if_id = gsk_spv_writer_make_id (writer);
after_id = gsk_spv_writer_make_id (writer);
@@ -581,7 +587,7 @@ gsk_sl_statement_for_write_spv (const GskSlStatement *statement,
gsk_spv_writer_branch (writer, condition_id);
gsk_spv_writer_start_code_block (writer, condition_id, continue_id, after_id);
gsk_spv_writer_label (writer, GSK_SPV_WRITER_SECTION_CODE, condition_id);
- test_id = gsk_sl_expression_write_spv (for_stmt->condition, writer);
+ test_id = gsk_sl_expression_write_spv (for_stmt->condition, writer, NULL);
gsk_spv_writer_branch_conditional (writer, test_id, body_id, after_id, NULL, 0);
}
@@ -592,7 +598,7 @@ gsk_sl_statement_for_write_spv (const GskSlStatement *statement,
gsk_spv_writer_label (writer, GSK_SPV_WRITER_SECTION_CODE, continue_id);
if (for_stmt->loop)
- gsk_sl_expression_write_spv (for_stmt->loop, writer);
+ gsk_sl_expression_write_spv (for_stmt->loop, writer, NULL);
gsk_spv_writer_branch (writer, loop_id);
gsk_spv_writer_start_code_block (writer, after_id, old_continue_id, old_break_id);
@@ -650,7 +656,7 @@ gsk_sl_statement_expression_write_spv (const GskSlStatement *statement,
{
GskSlStatementExpression *expression_statement = (GskSlStatementExpression *) statement;
- gsk_sl_expression_write_spv (expression_statement->expression, writer);
+ gsk_sl_expression_write_spv (expression_statement->expression, writer, NULL);
return FALSE;
}
@@ -1126,7 +1132,6 @@ its_a_type:
case GSK_SL_TOKEN_RETURN:
{
GskSlStatementReturn *return_statement;
- GskSlType *return_type;
if (!parse_everything)
goto only_expression_and_declaration;
@@ -1137,34 +1142,43 @@ its_a_type:
if (!gsk_sl_token_is (token, GSK_SL_TOKEN_SEMICOLON))
return_statement->value = gsk_sl_expression_parse (scope, preproc);
- return_type = gsk_sl_scope_get_return_type (scope);
+ return_statement->return_type = gsk_sl_scope_get_return_type (scope);
statement = (GskSlStatement *) return_statement;
- if (return_type == NULL)
+ if (return_statement->return_type == NULL)
{
gsk_sl_preprocessor_error (preproc, SCOPE, "Cannot return from here.");
+ gsk_sl_statement_unref (statement);
+ statement = gsk_sl_statement_new_error ();
}
- else if (return_statement->value == NULL)
- {
- if (!gsk_sl_type_is_void (return_type))
- {
- gsk_sl_preprocessor_error (preproc, TYPE_MISMATCH, "Functions expectes a return value of
type %s", gsk_sl_type_get_name (return_type));
- }
- }
- else
- {
- if (gsk_sl_type_is_void (return_type))
+ else
+ {
+ gsk_sl_type_ref (return_statement->return_type);
+
+ if (return_statement->value == NULL)
{
- gsk_sl_preprocessor_error (preproc, TYPE_MISMATCH, "Cannot return a value from a void
function.");
+ if (!gsk_sl_type_is_void (return_statement->return_type))
+ {
+ gsk_sl_preprocessor_error (preproc, TYPE_MISMATCH,
+ "Functions expectes a return value of type %s",
+ gsk_sl_type_get_name (return_statement->return_type));
+ }
}
- else if (!gsk_sl_type_can_convert (return_type, gsk_sl_expression_get_return_type
(return_statement->value)))
+ else
{
- gsk_sl_preprocessor_error (preproc, TYPE_MISMATCH,
- "Cannot convert type %s to return type %s.",
- gsk_sl_type_get_name (gsk_sl_expression_get_return_type
(return_statement->value)),
- gsk_sl_type_get_name (return_type));
- break;
- }
+ if (gsk_sl_type_is_void (return_statement->return_type))
+ {
+ gsk_sl_preprocessor_error (preproc, TYPE_MISMATCH, "Cannot return a value from a void
function.");
+ }
+ else if (!gsk_sl_type_can_convert (return_statement->return_type,
gsk_sl_expression_get_return_type (return_statement->value)))
+ {
+ gsk_sl_preprocessor_error (preproc, TYPE_MISMATCH,
+ "Cannot convert type %s to return type %s.",
+ gsk_sl_type_get_name (gsk_sl_expression_get_return_type
(return_statement->value)),
+ gsk_sl_type_get_name (return_statement->return_type));
+ break;
+ }
+ }
}
}
break;
diff --git a/gsk/gskspvwriter.c b/gsk/gskspvwriter.c
index 24a9f0c..b58f132 100644
--- a/gsk/gskspvwriter.c
+++ b/gsk/gskspvwriter.c
@@ -953,7 +953,7 @@ gsk_spv_access_resolve_pending (GskSpvAccessChain *chain)
if (g_array_index (chain->chain, guint32, i) != 0)
continue;
- g_array_index (chain->chain, guint32, i) = gsk_sl_expression_write_spv (l->data, chain->writer);
+ g_array_index (chain->chain, guint32, i) = gsk_sl_expression_write_spv (l->data, chain->writer, NULL);
l = l->next;
}
diff --git a/testsuite/gsksl/errors/invalid-return-type.glsl b/testsuite/gsksl/errors/invalid-return-type.glsl
new file mode 100644
index 0000000..742e13e
--- /dev/null
+++ b/testsuite/gsksl/errors/invalid-return-type.glsl
@@ -0,0 +1,11 @@
+float
+foo ()
+{
+ return vec3(0);
+}
+
+void
+main ()
+{
+ float f = foo();
+}
diff --git a/testsuite/gsksl/errors/returning-no-value.glsl b/testsuite/gsksl/errors/returning-no-value.glsl
new file mode 100644
index 0000000..1997afe
--- /dev/null
+++ b/testsuite/gsksl/errors/returning-no-value.glsl
@@ -0,0 +1,11 @@
+float
+foo ()
+{
+ return;
+}
+
+void
+main ()
+{
+ float f = foo();
+}
diff --git a/testsuite/gsksl/errors/returning-value-from-void.glsl
b/testsuite/gsksl/errors/returning-value-from-void.glsl
new file mode 100644
index 0000000..8eeeee1
--- /dev/null
+++ b/testsuite/gsksl/errors/returning-value-from-void.glsl
@@ -0,0 +1,11 @@
+void
+foo ()
+{
+ return 42;
+}
+
+void
+main ()
+{
+ foo();
+}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
