[PATCH vkd3d v2 5/5] vkd3d-shader: Make some FIXME messages into proper compiler errors.
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- Alternate approach I came up with, that I think I like better.
At least I'd like to keep printing messages to the console via FIXME; they're not going to show up otherwise.
libs/vkd3d-shader/hlsl.c | 24 +++++++++++++++--- libs/vkd3d-shader/hlsl.h | 9 +++++++ libs/vkd3d-shader/hlsl.y | 32 +++++++++++++----------- libs/vkd3d-shader/hlsl_sm1.c | 10 +++++--- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 5 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 7f12dd20..6d130358 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -55,6 +55,24 @@ void hlsl_warning(struct hlsl_ctx *ctx, const struct vkd3d_shader_location loc, va_end(args); }
+void hlsl_fixme_(struct hlsl_ctx *ctx, const struct vkd3d_shader_location loc, const char *fmt, ...) +{ + struct vkd3d_string_buffer *string; + va_list args; + + va_start(args, fmt); + string = hlsl_get_string_buffer(ctx); + vkd3d_string_buffer_printf(string, "Aborting due to not yet implemented feature: "); + vkd3d_string_buffer_vprintf(string, fmt, args); + string->buffer[--string->content_size] = 0; + vkd3d_shader_error(ctx->message_context, &loc, VKD3D_SHADER_ERROR_HLSL_NOT_IMPLEMENTED, "%s", string->buffer); + hlsl_release_string_buffer(ctx, string); + va_end(args); + + if (!ctx->result) + ctx->result = VKD3D_ERROR_NOT_IMPLEMENTED; +} + bool hlsl_add_var(struct hlsl_ctx *ctx, struct hlsl_ir_var *decl, bool local_var) { struct hlsl_scope *scope = ctx->cur_scope; @@ -1031,7 +1049,7 @@ static void dump_ir_constant(struct vkd3d_string_buffer *buffer, const struct hl vkd3d_string_buffer_printf(buffer, "}"); }
-static const char *debug_expr_op(const struct hlsl_ir_expr *expr) +const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op) { static const char *const op_names[] = { @@ -1082,14 +1100,14 @@ static const char *debug_expr_op(const struct hlsl_ir_expr *expr) [HLSL_OP3_LERP] = "lerp", };
- return op_names[expr->op]; + return op_names[op]; }
static void dump_ir_expr(struct vkd3d_string_buffer *buffer, const struct hlsl_ir_expr *expr) { unsigned int i;
- vkd3d_string_buffer_printf(buffer, "%s (", debug_expr_op(expr)); + vkd3d_string_buffer_printf(buffer, "%s (", debug_hlsl_expr_op(expr->op)); for (i = 0; i < 3 && expr->operands[i].node; ++i) { dump_src(buffer, &expr->operands[i]); diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 4a4af5d1..1405bef3 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -590,6 +590,7 @@ static inline void hlsl_release_string_buffer(struct hlsl_ctx *ctx, struct vkd3d vkd3d_string_buffer_release(&ctx->string_buffers, buffer); }
+const char *debug_hlsl_expr_op(enum hlsl_ir_expr_op op); const char *debug_hlsl_type(struct hlsl_ctx *ctx, const struct hlsl_type *type); const char *debug_hlsl_writemask(unsigned int writemask);
@@ -651,11 +652,19 @@ struct hlsl_ir_load *hlsl_new_var_load(struct hlsl_ctx *ctx, struct hlsl_ir_var
void hlsl_error(struct hlsl_ctx *ctx, const struct vkd3d_shader_location loc, enum vkd3d_shader_error error, const char *fmt, ...) VKD3D_PRINTF_FUNC(4, 5); +void hlsl_fixme_(struct hlsl_ctx *ctx, const struct vkd3d_shader_location loc, + const char *fmt, ...) VKD3D_PRINTF_FUNC(3, 4); void hlsl_warning(struct hlsl_ctx *ctx, const struct vkd3d_shader_location loc, enum vkd3d_shader_error error, const char *fmt, ...) VKD3D_PRINTF_FUNC(4, 5); void hlsl_note(struct hlsl_ctx *ctx, const struct vkd3d_shader_location loc, enum vkd3d_shader_log_level level, const char *fmt, ...) VKD3D_PRINTF_FUNC(4, 5);
+#define hlsl_fixme(ctx, loc, ...) \ + do { \ + FIXME(__VA_ARGS__); \ + hlsl_fixme_(ctx, loc, __VA_ARGS__); \ + } while (0) + void hlsl_push_scope(struct hlsl_ctx *ctx); void hlsl_pop_scope(struct hlsl_ctx *ctx);
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index ade78da6..73fc846c 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1213,7 +1213,7 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in { if (lhs->type == HLSL_IR_EXPR && hlsl_ir_expr(lhs)->op == HLSL_OP1_CAST) { - FIXME("Cast on the lhs.\n"); + hlsl_fixme(ctx, lhs->loc, "Cast on the LHS.\n"); vkd3d_free(store); return NULL; } @@ -1223,7 +1223,7 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in unsigned int width, s = swizzle->swizzle;
if (lhs->data_type->type == HLSL_CLASS_MATRIX) - FIXME("Assignments with writemasks and matrices on lhs are not supported yet.\n"); + hlsl_fixme(ctx, lhs->loc, "Matrix assignment with a writemask.\n");
if (!invert_swizzle(&s, &writemask, &width)) { @@ -1338,7 +1338,9 @@ static void struct_var_initializer(struct hlsl_ctx *ctx, struct list *list, stru list_add_tail(list, &store->node.entry); } else - FIXME("Initializing with "mismatched" fields is not supported yet.\n"); + { + hlsl_fixme(ctx, node->loc, "Implicit cast in structure initializer.\n"); + } }
vkd3d_free(initializer->args); @@ -1502,14 +1504,14 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t } if (v->arrays.count) { - FIXME("Initializing arrays is not supported yet.\n"); + hlsl_fixme(ctx, v->loc, "Array initializer.\n"); free_parse_initializer(&v->initializer); vkd3d_free(v); continue; } if (v->initializer.args_count > 1) { - FIXME("Complex initializers are not supported yet.\n"); + hlsl_fixme(ctx, v->loc, "Complex initializer.\n"); free_parse_initializer(&v->initializer); vkd3d_free(v); continue; @@ -1786,7 +1788,7 @@ hlsl_prog: | hlsl_prog declaration_statement { if (!list_empty($2)) - FIXME("Uniform initializer.\n"); + hlsl_fixme(ctx, @2, "Uniform initializer.\n"); hlsl_free_instr_list($2); } | hlsl_prog preproc_directive @@ -2769,7 +2771,7 @@ postfix_expr: }
if ($2->type == HLSL_CLASS_MATRIX) - FIXME("Matrix constructors are not supported yet.\n"); + hlsl_fixme(ctx, @2, "Matrix constructor.\n");
sprintf(name, "<constructor-%x>", counter++); if (!(var = hlsl_new_synthetic_var(ctx, name, $2, @2))) @@ -2931,11 +2933,11 @@ shift_expr: add_expr | shift_expr OP_LEFTSHIFT add_expr { - FIXME("Left shift.\n"); + hlsl_fixme(ctx, @$, "Left shift.\n"); } | shift_expr OP_RIGHTSHIFT add_expr { - FIXME("Right shift.\n"); + hlsl_fixme(ctx, @$, "Right shift.\n"); }
relational_expr: @@ -2972,42 +2974,42 @@ bitand_expr: equality_expr | bitand_expr '&' equality_expr { - FIXME("Bitwise AND.\n"); + hlsl_fixme(ctx, @$, "Bitwise AND.\n"); }
bitxor_expr: bitand_expr | bitxor_expr '^' bitand_expr { - FIXME("Bitwise XOR.\n"); + hlsl_fixme(ctx, @$, "Bitwise XOR.\n"); }
bitor_expr: bitxor_expr | bitor_expr '|' bitxor_expr { - FIXME("Bitwise OR.\n"); + hlsl_fixme(ctx, @$, "Bitwise OR.\n"); }
logicand_expr: bitor_expr | logicand_expr OP_AND bitor_expr { - FIXME("Logical AND.\n"); + hlsl_fixme(ctx, @$, "Logical AND.\n"); }
logicor_expr: logicand_expr | logicor_expr OP_OR logicand_expr { - FIXME("Logical OR.\n"); + hlsl_fixme(ctx, @$, "Logical OR.\n"); }
conditional_expr: logicor_expr | logicor_expr '?' expr ':' assignment_expr { - FIXME("Ternary operator.\n"); + hlsl_fixme(ctx, @$, "Ternary operator.\n"); }
assignment_expr: diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c index 85d54ac2..48a504da 100644 --- a/libs/vkd3d-shader/hlsl_sm1.c +++ b/libs/vkd3d-shader/hlsl_sm1.c @@ -606,7 +606,8 @@ static void write_sm1_expr(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b
if (instr->data_type->base_type != HLSL_TYPE_FLOAT) { - FIXME("Non-float operations need to be lowered.\n"); + /* These need to be lowered. */ + hlsl_fixme(ctx, instr->loc, "SM1 non-float expression.\n"); return; }
@@ -636,7 +637,7 @@ static void write_sm1_expr(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b break;
default: - FIXME("Unhandled op %u.\n", expr->op); + hlsl_fixme(ctx, instr->loc, "SM1 "%s" expression.\n", debug_hlsl_expr_op(expr->op)); break; } } @@ -765,8 +766,9 @@ static void write_sm1_instructions(struct hlsl_ctx *ctx, struct vkd3d_bytecode_b { if (instr->data_type->type == HLSL_CLASS_MATRIX) { - FIXME("Matrix operations need to be lowered.\n"); - break; + /* These need to be lowered. */ + hlsl_fixme(ctx, instr->loc, "SM1 matrix expression.\n"); + continue; }
assert(instr->data_type->type == HLSL_CLASS_SCALAR || instr->data_type->type == HLSL_CLASS_VECTOR); diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 9948045e..63fe9fc5 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -112,6 +112,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_HLSL_INVALID_RETURN = 5014, VKD3D_SHADER_ERROR_HLSL_OVERLAPPING_RESERVATIONS = 5015, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION = 5016, + VKD3D_SHADER_ERROR_HLSL_NOT_IMPLEMENTED = 5299,
VKD3D_SHADER_WARNING_HLSL_IMPLICIT_TRUNCATION = 5300, };
On Thu, 12 Aug 2021 at 06:47, Zebediah Figura zfigura@codeweavers.com wrote:
Alternate approach I came up with, that I think I like better.
At least I'd like to keep printing messages to the console via FIXME; they're not going to show up otherwise.
I'm fine with this; I mostly just care that these messages end up in the compiler output. That said, is the above really true? vkd3d-compiler is certainly going to print error messages by default, and presumably e.g. d3dcompiler would print them using its own debug facilities, similar to what wined3d does with vkd3d_shader_compile() (and e.g. GLSL compiler) messages.
@@ -112,6 +112,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_HLSL_INVALID_RETURN = 5014, VKD3D_SHADER_ERROR_HLSL_OVERLAPPING_RESERVATIONS = 5015, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION = 5016,
- VKD3D_SHADER_ERROR_HLSL_NOT_IMPLEMENTED = 5299,
The above is probably fine, but note that it would make it harder to later split the 5000-5299 range if we wanted to for some reason. At the same time, the HLSL error codes are not in any released version of vkd3d-shader yet, so you could still grab e.g. 5000 if we want a "nice" number for this.
On 8/12/21 10:53 AM, Henri Verbeet wrote:
On Thu, 12 Aug 2021 at 06:47, Zebediah Figura zfigura@codeweavers.com wrote:
Alternate approach I came up with, that I think I like better.
At least I'd like to keep printing messages to the console via FIXME; they're not going to show up otherwise.
I'm fine with this; I mostly just care that these messages end up in the compiler output. That said, is the above really true? vkd3d-compiler is certainly going to print error messages by default, and presumably e.g. d3dcompiler would print them using its own debug facilities, similar to what wined3d does with vkd3d_shader_compile() (and e.g. GLSL compiler) messages.
I was going to say that we probably don't want d3dcompiler to always print error messages, but on reflection it seems reasonable to print error messages iff vkd3d_shader_compile() returned an error. I'll get rid of the FIXME part.
@@ -112,6 +112,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_HLSL_INVALID_RETURN = 5014, VKD3D_SHADER_ERROR_HLSL_OVERLAPPING_RESERVATIONS = 5015, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION = 5016,
- VKD3D_SHADER_ERROR_HLSL_NOT_IMPLEMENTED = 5299,
The above is probably fine, but note that it would make it harder to later split the 5000-5299 range if we wanted to for some reason. At the same time, the HLSL error codes are not in any released version of vkd3d-shader yet, so you could still grab e.g. 5000 if we want a "nice" number for this.
I was just following Matteo's cue here; I don't really see any reason not to make it 5017. <shrug>
-
Henri Verbeet -
Zebediah Figura -
Zebediah Figura (she/her)