[Mesa-dev] [PATCH 14/18] i965: Replace reg_type_size[] with a function.
Kenneth Graunke
kenneth at whitecape.org
Sun Nov 27 09:26:28 UTC 2016
On Tuesday, November 22, 2016 11:59:48 AM PST Matt Turner wrote:
> A function is necessary to handle immediate types.
> ---
> src/mesa/drivers/dri/i965/brw_disasm.c | 35 ++++++++------------
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 58 +++++++++++++++++++++++++++++++--
> src/mesa/drivers/dri/i965/brw_reg.h | 8 +++++
> 3 files changed, 77 insertions(+), 24 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_disasm.c b/src/mesa/drivers/dri/i965/brw_disasm.c
> index 5e51be7..3786e4b 100644
> --- a/src/mesa/drivers/dri/i965/brw_disasm.c
> +++ b/src/mesa/drivers/dri/i965/brw_disasm.c
> @@ -257,20 +257,6 @@ static const char *const three_source_reg_encoding[] = {
> [BRW_3SRC_TYPE_DF] = "DF",
> };
>
> -const int reg_type_size[] = {
> - [BRW_HW_REG_TYPE_UD] = 4,
> - [BRW_HW_REG_TYPE_D] = 4,
> - [BRW_HW_REG_TYPE_UW] = 2,
> - [BRW_HW_REG_TYPE_W] = 2,
> - [BRW_HW_REG_NON_IMM_TYPE_UB] = 1,
> - [BRW_HW_REG_NON_IMM_TYPE_B] = 1,
> - [GEN7_HW_REG_NON_IMM_TYPE_DF] = 8,
> - [BRW_HW_REG_TYPE_F] = 4,
> - [GEN8_HW_REG_TYPE_UQ] = 8,
> - [GEN8_HW_REG_TYPE_Q] = 8,
> - [GEN8_HW_REG_NON_IMM_TYPE_HF] = 2,
> -};
> -
> static const char *const reg_file[4] = {
> [0] = "A",
> [1] = "g",
> @@ -734,6 +720,7 @@ reg(FILE *file, unsigned _reg_file, unsigned _reg_nr)
> static int
> dest(FILE *file, const struct gen_device_info *devinfo, brw_inst *inst)
> {
> + unsigned elem_size = brw_element_size(devinfo, inst, dst);
> int err = 0;
>
> if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) {
> @@ -744,7 +731,7 @@ dest(FILE *file, const struct gen_device_info *devinfo, brw_inst *inst)
> return 0;
> if (brw_inst_dst_da1_subreg_nr(devinfo, inst))
> format(file, ".%"PRIu64, brw_inst_dst_da1_subreg_nr(devinfo, inst) /
> - reg_type_size[brw_inst_dst_reg_type(devinfo, inst)]);
> + elem_size);
> string(file, "<");
> err |= control(file, "horiz stride", horiz_stride,
> brw_inst_dst_hstride(devinfo, inst), NULL);
> @@ -755,7 +742,7 @@ dest(FILE *file, const struct gen_device_info *devinfo, brw_inst *inst)
> string(file, "g[a0");
> if (brw_inst_dst_ia_subreg_nr(devinfo, inst))
> format(file, ".%"PRIu64, brw_inst_dst_ia_subreg_nr(devinfo, inst) /
> - reg_type_size[brw_inst_dst_reg_type(devinfo, inst)]);
> + elem_size);
> if (brw_inst_dst_ia1_addr_imm(devinfo, inst))
> format(file, " %d", brw_inst_dst_ia1_addr_imm(devinfo, inst));
> string(file, "]<");
> @@ -773,7 +760,7 @@ dest(FILE *file, const struct gen_device_info *devinfo, brw_inst *inst)
> return 0;
> if (brw_inst_dst_da16_subreg_nr(devinfo, inst))
> format(file, ".%"PRIu64, brw_inst_dst_da16_subreg_nr(devinfo, inst) /
> - reg_type_size[brw_inst_dst_reg_type(devinfo, inst)]);
> + elem_size);
> string(file, "<1>");
> err |= control(file, "writemask", writemask,
> brw_inst_da16_writemask(devinfo, inst), NULL);
> @@ -850,8 +837,10 @@ src_da1(FILE *file,
> err |= reg(file, _reg_file, reg_num);
> if (err == -1)
> return 0;
> - if (sub_reg_num)
> - format(file, ".%d", sub_reg_num / reg_type_size[type]); /* use formal style like spec */
> + if (sub_reg_num) {
> + unsigned elem_size = brw_hw_reg_type_to_size(devinfo, type, _reg_file);
> + format(file, ".%d", sub_reg_num / elem_size); /* use formal style like spec */
> + }
> src_align1_region(file, _vert_stride, _width, _horiz_stride);
> err |= control(file, "src reg encoding", reg_encoding, type, NULL);
> return err;
> @@ -936,10 +925,14 @@ src_da16(FILE *file,
> err |= reg(file, _reg_file, _reg_nr);
> if (err == -1)
> return 0;
> - if (_subreg_nr)
> + if (_subreg_nr) {
> + unsigned elem_size =
> + brw_hw_reg_type_to_size(devinfo, _reg_type, _reg_file);
> +
> /* bit4 for subreg number byte addressing. Make this same meaning as
> in da1 case, so output looks consistent. */
> - format(file, ".%d", 16 / reg_type_size[_reg_type]);
> + format(file, ".%d", 16 / elem_size);
> + }
> string(file, "<");
> err |= control(file, "vert stride", vert_stride, _vert_stride, NULL);
> string(file, ",4,1>");
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index f3aa2bc..de98102 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -141,6 +141,59 @@ brw_reg_type_to_hw_type(const struct gen_device_info *devinfo,
> }
> }
>
> +/**
> + * Return the element size given a hardware register type and file.
> + *
> + * The hardware encoding may depend on whether the value is an immediate.
> + */
> +unsigned
> +brw_hw_reg_type_to_size(const struct gen_device_info *devinfo,
> + unsigned type, enum brw_reg_file file)
> +{
> + if (file == BRW_IMMEDIATE_VALUE) {
> + static const int imm_hw_sizes[] = {
Maybe use unsigned, to match the function return type?
> + [BRW_HW_REG_TYPE_UD] = 4,
> + [BRW_HW_REG_TYPE_D] = 4,
> + [BRW_HW_REG_TYPE_UW] = 2,
> + [BRW_HW_REG_TYPE_W] = 2,
> + [BRW_HW_REG_IMM_TYPE_UV] = 2,
> + [BRW_HW_REG_IMM_TYPE_VF] = 4,
> + [BRW_HW_REG_IMM_TYPE_V] = 2,
> + [BRW_HW_REG_TYPE_F] = 4,
> + [GEN8_HW_REG_TYPE_UQ] = 8,
> + [GEN8_HW_REG_TYPE_Q] = 8,
> + [GEN8_HW_REG_IMM_TYPE_DF] = 8,
> + [GEN8_HW_REG_IMM_TYPE_HF] = 2,
> + };
> + assert(type < ARRAY_SIZE(imm_hw_sizes));
> + assert(imm_hw_sizes[type] != -1);
What's the point of these != -1 assertions? You've already checked that
it's one of the above array elements, and none of them are -1...perhaps
drop this?
> + assert(devinfo->gen >= 6 || type != BRW_HW_REG_IMM_TYPE_UV);
> + assert(devinfo->gen >= 8 || type <= BRW_HW_REG_TYPE_F);
> + return imm_hw_sizes[type];
> + } else {
> + /* Non-immediate registers */
> + static const int hw_sizes[] = {
ditto - unsigned?
> + [BRW_HW_REG_TYPE_UD] = 4,
> + [BRW_HW_REG_TYPE_D] = 4,
> + [BRW_HW_REG_TYPE_UW] = 2,
> + [BRW_HW_REG_TYPE_W] = 2,
> + [BRW_HW_REG_NON_IMM_TYPE_UB] = 1,
> + [BRW_HW_REG_NON_IMM_TYPE_B] = 1,
> + [GEN7_HW_REG_NON_IMM_TYPE_DF] = 8,
> + [BRW_HW_REG_TYPE_F] = 4,
> + [GEN8_HW_REG_TYPE_UQ] = 8,
> + [GEN8_HW_REG_TYPE_Q] = 8,
> + [GEN8_HW_REG_NON_IMM_TYPE_HF] = 2,
> + };
> + assert(type < ARRAY_SIZE(hw_sizes));
> + assert(hw_sizes[type] != -1);
ditto - drop this assert?
Either way,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> + assert(devinfo->gen >= 7 ||
> + (type < GEN7_HW_REG_NON_IMM_TYPE_DF || type == BRW_HW_REG_TYPE_F));
> + assert(devinfo->gen >= 8 || type <= BRW_HW_REG_TYPE_F);
> + return hw_sizes[type];
> + }
> +}
> +
> void
> brw_set_dest(struct brw_codegen *p, brw_inst *inst, struct brw_reg dest)
> {
> @@ -219,8 +272,6 @@ brw_set_dest(struct brw_codegen *p, brw_inst *inst, struct brw_reg dest)
> brw_inst_set_exec_size(devinfo, inst, dest.width);
> }
>
> -extern int reg_type_size[];
> -
> static void
> validate_reg(const struct gen_device_info *devinfo,
> brw_inst *inst, struct brw_reg reg)
> @@ -237,8 +288,9 @@ validate_reg(const struct gen_device_info *devinfo,
> * destination horiz stride has to be a word.
> */
> if (reg.type == BRW_REGISTER_TYPE_V) {
> + unsigned elem_size = brw_element_size(devinfo, inst, dst);
> assert(hstride_for_reg[brw_inst_dst_hstride(devinfo, inst)] *
> - reg_type_size[brw_inst_dst_reg_type(devinfo, inst)] == 2);
> + elem_size == 2);
> }
>
> return;
> diff --git a/src/mesa/drivers/dri/i965/brw_reg.h b/src/mesa/drivers/dri/i965/brw_reg.h
> index 8907c9c..f9e1b37 100644
> --- a/src/mesa/drivers/dri/i965/brw_reg.h
> +++ b/src/mesa/drivers/dri/i965/brw_reg.h
> @@ -225,6 +225,14 @@ enum PACKED brw_reg_type {
>
> unsigned brw_reg_type_to_hw_type(const struct gen_device_info *devinfo,
> enum brw_reg_type type, enum brw_reg_file file);
> +
> +#define brw_element_size(devinfo, inst, operand) \
> + brw_hw_reg_type_to_size(devinfo, \
> + brw_inst_ ## operand ## _reg_type(devinfo, inst), \
> + brw_inst_ ## operand ## _reg_file(devinfo, inst))
> +unsigned brw_hw_reg_type_to_size(const struct gen_device_info *devinfo,
> + unsigned type, enum brw_reg_file file);
> +
> const char *brw_reg_type_letters(unsigned brw_reg_type);
> uint32_t brw_swizzle_immediate(enum brw_reg_type type, uint32_t x, unsigned swz);
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161127/aca1b40f/attachment.sig>
More information about the mesa-dev
mailing list