This seems generally fine to me. Couple of points:
-void shader_instruction_init(struct vkd3d_shader_instruction *ins, enum vkd3d_shader_opcode handler_idx) +void shader_instruction_init(struct vkd3d_shader_instruction *ins, const struct vkd3d_shader_location *location, + enum vkd3d_shader_opcode handler_idx)
If we're touching it anyway, we might as well rename it to vsir_instruction_init(), IMO.
@@ -1062,7 +1068,10 @@ static void shader_instruction_normalise_io_params(struct vkd3d_shader_instructi } if (!keep) - shader_instruction_init(ins, VKD3DSIH_NOP); + { + location = ins->location; + shader_instruction_init(ins, &location, VKD3DSIH_NOP); + } } static enum vkd3d_result instruction_array_normalise_io_registers(struct vkd3d_shader_instruction_array *instructions,
This is not introduced by this series, so in that regard it's fine, but this looks odd. In particular:
- Why shader_instruction_init(..., VKD3DSIH_NOP) instead of vkd3d_shader_instruction_make_nop()? - Why the "keep" variable if we could just nop out the instruction in the two places that might set it to false?
+void vkd3d_shader_validate_vsir(struct vkd3d_shader_parser *parser)
I.e., vsir_validate()? :)
+static void validator_error(struct validation_context *ctx, enum vkd3d_shader_error error, + const char *format, ...) VKD3D_PRINTF_FUNC(3, 4); + +static void validator_error(struct validation_context *ctx, enum vkd3d_shader_error error, + const char *format, ...) +{
I.e.,
```c static void VKD3D_PRINTF_FUNC(3, 4) validator_error(struct validation_context *ctx, enum vkd3d_shader_error error, const char *format, ...) { ... ```
right?
+ case VKD3DSIH_INVALID: + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_HANDLER, "Invalid handler.\n");
Note that vkd3d_shader_verror() already adds a newline, so validator_error() shouldn't need to add an extra one. That happens in a few other places in this series as well.
+static void vkd3d_shader_validate_register(struct validation_context *ctx, + const struct vkd3d_shader_register *reg) +{ + switch (reg->type) + { + case VKD3DSPR_COUNT: + case VKD3DSPR_INVALID: + validator_error(ctx, VKD3D_SHADER_ERROR_VSIR_INVALID_REGISTER_TYPE, "Invalid register type.\n"); + break; + + default: + break; + } +}
Should we just check for "reg->type >= VKD3DSPR_COUNT" and print the actual value here?