Module: Mesa Branch: main Commit: 1bd84f475363cfeebcd950f0cb9bb57d6900bdbf URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=1bd84f475363cfeebcd950f0cb9bb57d6900bdbf
Author: Simon Perretta <[email protected]> Date: Wed Jan 18 15:41:23 2023 +0000 pvr: Validate instruction repeat and src/dst sizes Signed-off-by: Simon Perretta <[email protected]> Acked-by: Frank Binns <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20430> --- src/imagination/rogue/rogue.h | 72 +++++++++--- src/imagination/rogue/rogue_info.c | 28 ++++- src/imagination/rogue/rogue_validate.c | 194 ++++++++++++++++++++++++++------- 3 files changed, 237 insertions(+), 57 deletions(-) diff --git a/src/imagination/rogue/rogue.h b/src/imagination/rogue/rogue.h index ef81bf9af1f..b05715719a4 100644 --- a/src/imagination/rogue/rogue.h +++ b/src/imagination/rogue/rogue.h @@ -37,17 +37,12 @@ /* TODO NEXT: Make things const that can be. */ /* TODO NEXT: Use debug message functions in u_debug.h */ -/* TODO NEXT: for validating `repeat` values, add to rogue instr info which - * srcs/dsts will be affected and will need to be checked */ /* TODO NEXT: Reg/src/dst things are often just simple comparisons; write * per-instr-type comparison function that also checks things like modifiers. */ /* TODO NEXT: In "unreachable"s, replace "invalid" with "unsupported". */ -/* TODO NEXT: go through things that return bool and ensure they return 1 or 0, - * not a mask (whose first bit may be non-zero)! */ - #include "compiler/nir/nir.h" #include "compiler/shader_enums.h" #include "compiler/spirv/nir_spirv.h" @@ -756,8 +751,7 @@ static inline enum rogue_reg_class rogue_ref_get_reg_class(const rogue_ref *ref) return ref->regarray->regs[0]->class; else if (rogue_ref_is_reg(ref)) return ref->reg->class; - else - unreachable("Ref is not a reg/regarray."); + unreachable("Ref is not a reg/regarray."); } static inline unsigned rogue_ref_get_reg_index(const rogue_ref *ref) @@ -766,8 +760,14 @@ static inline unsigned rogue_ref_get_reg_index(const rogue_ref *ref) return ref->regarray->regs[0]->index; else if (rogue_ref_is_reg(ref)) return ref->reg->index; - else - unreachable("Ref is not a reg/regarray."); + unreachable("Ref is not a reg/regarray."); +} + +static inline unsigned rogue_ref_get_regarray_size(const rogue_ref *ref) +{ + if (rogue_ref_is_regarray(ref)) + return ref->regarray->size; + unreachable("Ref is not a regarray."); } #define ROGUE_INTERNAL0_OFFSET 36 @@ -1046,6 +1046,9 @@ typedef struct rogue_ctrl_op_mod_info { extern const rogue_ctrl_op_mod_info rogue_ctrl_op_mod_infos[ROGUE_CTRL_OP_MOD_COUNT]; +#define ROGUE_CTRL_OP_MAX_SRCS 7 +#define ROGUE_CTRL_OP_MAX_DSTS 2 + typedef struct rogue_ctrl_op_info { const char *str; @@ -1058,6 +1061,17 @@ typedef struct rogue_ctrl_op_info { unsigned num_srcs; uint64_t supported_op_mods; + uint64_t supported_dst_mods[ROGUE_CTRL_OP_MAX_DSTS]; + uint64_t supported_src_mods[ROGUE_CTRL_OP_MAX_SRCS]; + + uint64_t supported_dst_types[ROGUE_CTRL_OP_MAX_DSTS]; + uint64_t supported_src_types[ROGUE_CTRL_OP_MAX_SRCS]; + + unsigned dst_stride[ROGUE_CTRL_OP_MAX_DSTS]; + unsigned src_stride[ROGUE_CTRL_OP_MAX_SRCS]; + + uint64_t dst_repeat_mask; + uint64_t src_repeat_mask; } rogue_ctrl_op_info; extern const rogue_ctrl_op_info rogue_ctrl_op_infos[ROGUE_CTRL_OP_COUNT]; @@ -1097,9 +1111,14 @@ typedef struct rogue_alu_op_info { uint64_t supported_dst_mods[ROGUE_ALU_OP_MAX_DSTS]; uint64_t supported_src_mods[ROGUE_ALU_OP_MAX_SRCS]; - /* TODO NEXT: Do the same for other instruction types. */ uint64_t supported_dst_types[ROGUE_ALU_OP_MAX_DSTS]; uint64_t supported_src_types[ROGUE_ALU_OP_MAX_SRCS]; + + unsigned dst_stride[ROGUE_CTRL_OP_MAX_DSTS]; + unsigned src_stride[ROGUE_CTRL_OP_MAX_SRCS]; + + uint64_t dst_repeat_mask; + uint64_t src_repeat_mask; } rogue_alu_op_info; extern const rogue_alu_op_info rogue_alu_op_infos[ROGUE_ALU_OP_COUNT]; @@ -1270,6 +1289,17 @@ typedef struct rogue_backend_op_info { rogue_backend_io_info phase_io; uint64_t supported_op_mods; + uint64_t supported_dst_mods[ROGUE_BACKEND_OP_MAX_DSTS]; + uint64_t supported_src_mods[ROGUE_BACKEND_OP_MAX_SRCS]; + + uint64_t supported_dst_types[ROGUE_BACKEND_OP_MAX_DSTS]; + uint64_t supported_src_types[ROGUE_BACKEND_OP_MAX_SRCS]; + + unsigned dst_stride[ROGUE_CTRL_OP_MAX_DSTS]; + unsigned src_stride[ROGUE_CTRL_OP_MAX_SRCS]; + + uint64_t dst_repeat_mask; + uint64_t src_repeat_mask; } rogue_backend_op_info; extern const rogue_backend_op_info @@ -1321,9 +1351,6 @@ rogue_backend_op_mod_is_set(const rogue_backend_instr *backend, rogue_backend_instr *rogue_backend_instr_create(rogue_block *block, enum rogue_backend_op op); -#define ROGUE_CTRL_OP_MAX_SRCS 7 -#define ROGUE_CTRL_OP_MAX_DSTS 2 - typedef struct rogue_ctrl_instr { rogue_instr instr; @@ -1378,21 +1405,32 @@ enum rogue_bitwise_op { ROGUE_BITWISE_OP_COUNT, }; +#define ROGUE_BITWISE_OP_MAX_SRCS 7 +#define ROGUE_BITWISE_OP_MAX_DSTS 2 + typedef struct rogue_bitwise_op_info { const char *str; unsigned num_dsts; unsigned num_srcs; - /* uint64_t supported_op_mods; */ + uint64_t supported_op_mods; + uint64_t supported_dst_mods[ROGUE_BITWISE_OP_MAX_DSTS]; + uint64_t supported_src_mods[ROGUE_BITWISE_OP_MAX_SRCS]; + + uint64_t supported_dst_types[ROGUE_BITWISE_OP_MAX_DSTS]; + uint64_t supported_src_types[ROGUE_BITWISE_OP_MAX_SRCS]; + + unsigned dst_stride[ROGUE_CTRL_OP_MAX_DSTS]; + unsigned src_stride[ROGUE_CTRL_OP_MAX_SRCS]; + + uint64_t dst_repeat_mask; + uint64_t src_repeat_mask; } rogue_bitwise_op_info; extern const rogue_bitwise_op_info rogue_bitwise_op_infos[ROGUE_BITWISE_OP_COUNT]; -#define ROGUE_BITWISE_OP_MAX_SRCS 7 -#define ROGUE_BITWISE_OP_MAX_DSTS 2 - typedef struct rogue_bitwise_dst { rogue_ref ref; unsigned index; diff --git a/src/imagination/rogue/rogue_info.c b/src/imagination/rogue/rogue_info.c index 55ed81e99a5..2b7ee1e3abc 100644 --- a/src/imagination/rogue/rogue_info.c +++ b/src/imagination/rogue/rogue_info.c @@ -221,6 +221,7 @@ const rogue_ctrl_op_mod_info rogue_ctrl_op_mod_infos[ROGUE_CTRL_OP_MOD_COUNT] = }; #define OM(op_mod) BITFIELD64_BIT(ROGUE_CTRL_OP_MOD_##op_mod) +#define T(type) BITFIELD64_BIT(ROGUE_REF_TYPE_##type - 1) const rogue_ctrl_op_info rogue_ctrl_op_infos[ROGUE_CTRL_OP_COUNT] = { [ROGUE_CTRL_OP_INVALID] = { .str = "!INVALID!", }, [ROGUE_CTRL_OP_END] = { .str = "end", .ends_block = true, }, @@ -228,16 +229,23 @@ const rogue_ctrl_op_info rogue_ctrl_op_infos[ROGUE_CTRL_OP_COUNT] = { .supported_op_mods = OM(END), }, [ROGUE_CTRL_OP_BA] = { .str = "ba", .has_target = true, .ends_block = true, }, - [ROGUE_CTRL_OP_WDF] = { .str = "wdf", .num_srcs = 1, }, + [ROGUE_CTRL_OP_WDF] = { .str = "wdf", .num_srcs = 1, + .supported_src_types = { [0] = T(DRC), }, + }, }; +#undef T #undef OM #define IO(io) ROGUE_IO_##io #define OM(op_mod) BITFIELD64_BIT(ROGUE_BACKEND_OP_MOD_##op_mod) +#define T(type) BITFIELD64_BIT(ROGUE_REF_TYPE_##type - 1) +#define B(n) BITFIELD64_BIT(n) const rogue_backend_op_info rogue_backend_op_infos[ROGUE_BACKEND_OP_COUNT] = { [ROGUE_BACKEND_OP_INVALID] = { .str = "!INVALID!", }, [ROGUE_BACKEND_OP_UVSW_WRITE] = { .str = "uvsw.write", .num_dsts = 1, .num_srcs = 1, .phase_io = { .src[0] = IO(W0), }, + .supported_dst_types = { [0] = T(REG), }, + .supported_src_types = { [0] = T(REG), }, }, [ROGUE_BACKEND_OP_UVSW_EMIT] = { .str = "uvsw.emit", }, [ROGUE_BACKEND_OP_UVSW_ENDTASK] = { .str = "uvsw.endtask", }, @@ -245,12 +253,27 @@ const rogue_backend_op_info rogue_backend_op_infos[ROGUE_BACKEND_OP_COUNT] = { [ROGUE_BACKEND_OP_UVSW_EMITTHENENDTASK] = { .str = "uvsw.emitthenendtask", }, [ROGUE_BACKEND_OP_UVSW_WRITETHENEMITTHENENDTASK] = { .str = "uvsw.writethenemitthenendtask", .num_dsts = 1, .num_srcs = 1, .phase_io = { .src[0] = IO(W0), }, + .supported_dst_types = { [0] = T(REG), }, + .supported_src_types = { [0] = T(REG), }, }, [ROGUE_BACKEND_OP_FITRP_PIXEL] = { .str = "fitrp.pixel", .num_dsts = 1, .num_srcs = 4, .phase_io = { .dst[0] = IO(S3), .src[1] = IO(S0), .src[2] = IO(S2), }, .supported_op_mods = OM(SAT), + .supported_dst_types = { [0] = T(REG), }, + .supported_src_types = { + [0] = T(DRC), + [1] = T(REGARRAY), + [2] = T(REGARRAY), + [3] = T(VAL), + }, + .src_stride = { + [1] = 3, + [2] = 3, + }, }, }; +#undef B +#undef T #undef OM #undef IO @@ -298,6 +321,7 @@ const rogue_io_info rogue_io_infos[ROGUE_IO_COUNT] = { #define PH(type) ROGUE_INSTR_PHASE_##type #define IO(io) ROGUE_IO_##io #define T(type) BITFIELD64_BIT(ROGUE_REF_TYPE_##type - 1) +#define B(n) BITFIELD64_BIT(n) const rogue_alu_op_info rogue_alu_op_infos[ROGUE_ALU_OP_COUNT] = { [ROGUE_ALU_OP_INVALID] = { .str = "!INVALID!", }, [ROGUE_ALU_OP_MBYP] = { .str = "mbyp", .num_dsts = 1, .num_srcs = 1, @@ -360,6 +384,7 @@ const rogue_alu_op_info rogue_alu_op_infos[ROGUE_ALU_OP_COUNT] = { .supported_src_types = { [0] = T(REGARRAY), }, + .src_repeat_mask = B(0), }, /* This mov is "fake" since it can be lowered to a MBYP, make a new instruction for real mov (call it MOVD?). */ [ROGUE_ALU_OP_MOV] = { .str = "mov", .num_dsts = 1, .num_srcs = 1, @@ -376,6 +401,7 @@ const rogue_alu_op_info rogue_alu_op_infos[ROGUE_ALU_OP_COUNT] = { [ROGUE_ALU_OP_FMIN] = { .str = "fmin", .num_dsts = 1, .num_srcs = 2, }, /* TODO */ [ROGUE_ALU_OP_SEL] = { .str = "sel", .num_dsts = 1, .num_srcs = 3, }, /* TODO */ }; +#undef B #undef T #undef IO #undef PH diff --git a/src/imagination/rogue/rogue_validate.c b/src/imagination/rogue/rogue_validate.c index e8faf433fd5..6c87b3fc691 100644 --- a/src/imagination/rogue/rogue_validate.c +++ b/src/imagination/rogue/rogue_validate.c @@ -41,9 +41,6 @@ * through ISR and add any other restrictions). */ /* TODO: Remember that some instructions have the DESTINATION as a source * (register pointers), e.g. fitrp using S3 */ -/* TODO NEXT: Add field to instr_info that specifies which source/destination - * should be affected by instruction repeating. */ -/* TODO NEXT: Validate backend and control sources/dests. */ /* TODO: Go through and make sure that validation state is being properly * updated as so to allow for validation_log to print enough info. */ @@ -54,10 +51,14 @@ typedef struct rogue_validation_state { const rogue_shader *shader; /** The shader being validated. */ const char *when; /** Description of the validation being done. */ bool nonfatal; /** Don't stop at the first error.*/ - const rogue_instr *instr; /** Current instruction being validated. */ - const rogue_instr_group *group; /** Current instruction group being - validated. */ - const rogue_ref *ref; /** Current reference being validated. */ + struct { + const rogue_instr *instr; /** Current instruction being validated. */ + const rogue_instr_group *group; /** Current instruction group being + validated. */ + const rogue_ref *ref; /** Current reference being validated. */ + bool src; /** Current reference type (src/dst). */ + unsigned param; /** Current reference src/dst index. */ + } ctx; struct util_dynarray *error_msgs; /** Error message list. */ } rogue_validation_state; @@ -73,7 +74,6 @@ static bool validate_print_errors(rogue_validation_state *state) fputs("\n", stderr); - /* TODO: Figure out if/when to print this. */ rogue_print_shader(stderr, state->shader); fputs("\n", stderr); @@ -86,12 +86,15 @@ static void PRINTFLIKE(2, 3) char *msg = ralloc_asprintf(state->error_msgs, "Validation error"); /* Add info about the item that was being validated. */ - if (state->instr) { - ralloc_asprintf_append(&msg, " instr %u", state->instr->index); + if (state->ctx.instr) { + ralloc_asprintf_append(&msg, " instr %u", state->ctx.instr->index); } - if (state->ref) { - /* TODO: Find a way to get an index. */ + if (state->ctx.ref) { + ralloc_asprintf_append(&msg, + " %s %u", + state->ctx.src ? "src" : "dst", + state->ctx.param); } ralloc_asprintf_append(&msg, ": "); @@ -143,37 +146,80 @@ static void validate_regarray(rogue_validation_state *state, } } -static void validate_alu_dst(rogue_validation_state *state, - const rogue_instr_dst *dst, - uint64_t supported_dst_types) +static void validate_dst(rogue_validation_state *state, + const rogue_instr_dst *dst, + uint64_t supported_dst_types, + unsigned i, + unsigned stride, + unsigned repeat, + uint64_t repeat_mask) { - state->ref = &dst->ref; + state->ctx.ref = &dst->ref; + state->ctx.src = false; + state->ctx.param = i; if (rogue_ref_is_null(&dst->ref)) - validate_log(state, "ALU destination has not been set."); + validate_log(state, "Destination has not been set."); + + if (!state->shader->is_grouped) { + unsigned dst_size = stride + 1; + if (repeat_mask & (1 << i)) + dst_size *= repeat; + + if (rogue_ref_is_regarray(&dst->ref)) { + if (rogue_ref_get_regarray_size(&dst->ref) != dst_size) { + validate_log(state, + "Expected regarray size %u, got %u.", + dst_size, + rogue_ref_get_regarray_size(&dst->ref)); + } + } else if (dst_size > 1) { + validate_log(state, "Expected regarray type for destination."); + } - if (!state->shader->is_grouped) if (!rogue_ref_type_supported(dst->ref.type, supported_dst_types)) - validate_log(state, "Unsupported ALU destination type."); + validate_log(state, "Unsupported destination type."); + } - state->ref = NULL; + state->ctx.ref = NULL; } -static void validate_alu_src(rogue_validation_state *state, - const rogue_instr_src *src, - uint64_t supported_src_types) +static void validate_src(rogue_validation_state *state, + const rogue_instr_src *src, + uint64_t supported_src_types, + unsigned i, + unsigned stride, + unsigned repeat, + uint64_t repeat_mask) { - state->ref = &src->ref; + state->ctx.ref = &src->ref; + state->ctx.src = true; + state->ctx.param = i; if (rogue_ref_is_null(&src->ref)) - validate_log(state, "ALU source has not been set."); + validate_log(state, "Source has not been set."); if (!state->shader->is_grouped) { + unsigned src_size = stride + 1; + if (repeat_mask & (1 << i)) + src_size *= repeat; + + if (rogue_ref_is_regarray(&src->ref)) { + if (rogue_ref_get_regarray_size(&src->ref) != src_size) { + validate_log(state, + "Expected regarray size %u, got %u.", + src_size, + rogue_ref_get_regarray_size(&src->ref)); + } + } else if (src_size > 1) { + validate_log(state, "Expected regarray type for source."); + } + if (!rogue_ref_type_supported(src->ref.type, supported_src_types)) - validate_log(state, "Unsupported ALU source type."); + validate_log(state, "Unsupported source type."); } - state->ref = NULL; + state->ctx.ref = NULL; } static void validate_alu_instr(rogue_validation_state *state, @@ -194,12 +240,32 @@ static void validate_alu_instr(rogue_validation_state *state, if (!rogue_mods_supported(alu->mod, info->supported_op_mods)) validate_log(state, "Unsupported ALU op modifiers."); + /* Instruction repeat checks. */ + if (alu->instr.repeat > 1 && !info->dst_repeat_mask && + !info->src_repeat_mask) { + validate_log(state, "Repeat set for ALU op without repeat support."); + } + /* Validate destinations and sources. */ - for (unsigned i = 0; i < info->num_dsts; ++i) - validate_alu_dst(state, &alu->dst[i], info->supported_dst_types[i]); + for (unsigned i = 0; i < info->num_dsts; ++i) { + validate_dst(state, + &alu->dst[i], + info->supported_dst_types[i], + i, + info->dst_stride[i], + alu->instr.repeat, + info->dst_repeat_mask); + } - for (unsigned i = 0; i < info->num_srcs; ++i) - validate_alu_src(state, &alu->src[i], info->supported_src_types[i]); + for (unsigned i = 0; i < info->num_srcs; ++i) { + validate_src(state, + &alu->src[i], + info->supported_src_types[i], + i, + info->src_stride[i], + alu->instr.repeat, + info->src_repeat_mask); + } /* TODO: Check that the src_use and dst_write fields are correct? */ } @@ -217,7 +283,33 @@ static void validate_backend_instr(rogue_validation_state *state, if (!rogue_mods_supported(backend->mod, info->supported_op_mods)) validate_log(state, "Unsupported backend op modifiers."); - /* TODO: Validate dests and srcs? */ + /* Instruction repeat checks. */ + if (backend->instr.repeat > 1 && !info->dst_repeat_mask && + !info->src_repeat_mask) { + validate_log(state, "Repeat set for backend op without repeat support."); + } + + /* Validate destinations and sources. */ + for (unsigned i = 0; i < info->num_dsts; ++i) { + validate_dst(state, + &backend->dst[i], + info->supported_dst_types[i], + i, + info->dst_stride[i], + backend->instr.repeat, + info->dst_repeat_mask); + } + + for (unsigned i = 0; i < info->num_srcs; ++i) { + validate_src(state, + &backend->src[i], + info->supported_src_types[i], + i, + info->src_stride[i], + backend->instr.repeat, + info->src_repeat_mask); + } + /* TODO: Check that the src_use and dst_write fields are correct? */ } @@ -241,7 +333,33 @@ static bool validate_ctrl_instr(rogue_validation_state *state, if (!rogue_mods_supported(ctrl->mod, info->supported_op_mods)) validate_log(state, "Unsupported CTRL op modifiers."); - /* TODO: Validate dests and srcs? */ + /* Instruction repeat checks. */ + if (ctrl->instr.repeat > 1 && !info->dst_repeat_mask && + !info->src_repeat_mask) { + validate_log(state, "Repeat set for CTRL op without repeat support."); + } + + /* Validate destinations and sources. */ + for (unsigned i = 0; i < info->num_dsts; ++i) { + validate_dst(state, + &ctrl->dst[i], + info->supported_dst_types[i], + i, + info->dst_stride[i], + ctrl->instr.repeat, + info->dst_repeat_mask); + } + + for (unsigned i = 0; i < info->num_srcs; ++i) { + validate_src(state, + &ctrl->src[i], + info->supported_src_types[i], + i, + info->src_stride[i], + ctrl->instr.repeat, + info->src_repeat_mask); + } + /* TODO: Check that the src_use and dst_write fields are correct? */ /* nop.end counts as a end-of-block instruction. */ @@ -259,7 +377,7 @@ static bool validate_ctrl_instr(rogue_validation_state *state, static bool validate_instr(rogue_validation_state *state, const rogue_instr *instr) { - state->instr = instr; + state->ctx.instr = instr; bool ends_block = false; @@ -287,7 +405,7 @@ static bool validate_instr(rogue_validation_state *state, if (!ends_block) ends_block = instr->end; - state->instr = NULL; + state->ctx.instr = NULL; return ends_block; } @@ -296,7 +414,7 @@ static bool validate_instr(rogue_validation_state *state, static bool validate_instr_group(rogue_validation_state *state, const rogue_instr_group *group) { - state->group = group; + state->ctx.group = group; /* TODO: Validate group properties. */ /* TODO: Check for pseudo-instructions. */ @@ -315,7 +433,7 @@ static bool validate_instr_group(rogue_validation_state *state, ends_block = validate_instr(state, instr); } - state->group = NULL; + state->ctx.group = NULL; if (group->header.alu != ROGUE_ALU_CONTROL) return group->header.end; @@ -485,8 +603,6 @@ static void validate_reg_state(rogue_validation_state *state, } } -/* TODO: To properly test this and see what needs validating, try and write some - * failing tests and then filling them from there. */ PUBLIC bool rogue_validate_shader(rogue_shader *shader, const char *when) {
