Matt Turner <matts...@gmail.com> writes:
> These vaguely corresponded to the hardware encodings, but that is purely
> historical at this point. Reorder them so we stop making things "almost
> work" when mixing enums.
>
> The ordering has been closen so that no enum value is the same as a
> compatible hardware encoding.
> ---
> src/intel/compiler/brw_eu.c | 1 -
> src/intel/compiler/brw_eu_emit.c | 6 ------
> src/intel/compiler/brw_fs.cpp | 1 +
> src/intel/compiler/brw_reg.h | 32 ++++++++++++++------------------
> src/intel/compiler/brw_vec4.cpp | 3 ++-
> 5 files changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c
> index 0ef52e219c..700a1badd4 100644
> --- a/src/intel/compiler/brw_eu.c
> +++ b/src/intel/compiler/brw_eu.c
> @@ -62,7 +62,6 @@ brw_reg_type_letters(unsigned type)
> [BRW_REGISTER_TYPE_UQ] = "UQ",
> [BRW_REGISTER_TYPE_Q] = "Q",
> };
> - assert(type <= BRW_REGISTER_TYPE_Q);
> return names[type];
> }
>
> diff --git a/src/intel/compiler/brw_eu_emit.c
> b/src/intel/compiler/brw_eu_emit.c
> index 6673e0741a..b59fc33a54 100644
> --- a/src/intel/compiler/brw_eu_emit.c
> +++ b/src/intel/compiler/brw_eu_emit.c
> @@ -112,7 +112,6 @@ brw_reg_type_to_hw_type(const struct gen_device_info
> *devinfo,
> };
> assert(type < ARRAY_SIZE(imm_hw_types));
> assert(imm_hw_types[type] != -1);
> - assert(devinfo->gen >= 8 || type < BRW_REGISTER_TYPE_DF);
> return imm_hw_types[type];
> } else {
> /* Non-immediate registers */
> @@ -134,8 +133,6 @@ brw_reg_type_to_hw_type(const struct gen_device_info
> *devinfo,
> };
> assert(type < ARRAY_SIZE(hw_types));
> assert(hw_types[type] != -1);
> - assert(devinfo->gen >= 7 || type < BRW_REGISTER_TYPE_DF);
> - assert(devinfo->gen >= 8 || type < BRW_REGISTER_TYPE_Q);
It might be good to keep these asserts around, but phrased as something
like assert(gen_has_reg_type(gen, file, type)) or something.
> return hw_types[type];
> }
> }
> @@ -184,9 +181,6 @@ brw_hw_reg_type_to_size(const struct gen_device_info
> *devinfo,
> [GEN8_HW_REG_NON_IMM_TYPE_HF] = 2,
> };
> assert(type < ARRAY_SIZE(hw_sizes));
> - 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];
> }
> }
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index fdc30d450c..0ea4c4f1cc 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -403,6 +403,7 @@ void
> fs_reg::init()
> {
> memset(this, 0, sizeof(*this));
> + type = BRW_REGISTER_TYPE_UD;
> stride = 1;
> }
>
> diff --git a/src/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
> index 17a51fbd65..48e6fd7b7d 100644
> --- a/src/intel/compiler/brw_reg.h
> +++ b/src/intel/compiler/brw_reg.h
> @@ -203,29 +203,25 @@ brw_mask_for_swizzle(unsigned swz)
> }
>
> enum PACKED brw_reg_type {
And maybe add a comment here like this part of the commit message:
* The ordering has been chosen so that no enum value is the same as a
* compatible hardware encoding.
So it's clear that matching some hardware encoding is an anti-goal.
fwiw:
Reviewed-by: Scott D Phillips <scott.d.phill...@intel.com>
> - BRW_REGISTER_TYPE_UD = 0,
> - BRW_REGISTER_TYPE_D,
> - BRW_REGISTER_TYPE_UW,
> - BRW_REGISTER_TYPE_W,
> + /** Floating-point types: @{ */
> + BRW_REGISTER_TYPE_DF,
> BRW_REGISTER_TYPE_F,
> -
> - /** Non-immediates only: @{ */
> - BRW_REGISTER_TYPE_UB,
> - BRW_REGISTER_TYPE_B,
> - /** @} */
> -
> - /** Immediates only: @{ */
> - BRW_REGISTER_TYPE_UV, /* Gen6+ */
> - BRW_REGISTER_TYPE_V,
> + BRW_REGISTER_TYPE_HF,
> BRW_REGISTER_TYPE_VF,
> /** @} */
>
> - BRW_REGISTER_TYPE_DF, /* Gen7+ (no immediates until Gen8+) */
> -
> - /* Gen8+ */
> - BRW_REGISTER_TYPE_HF,
> - BRW_REGISTER_TYPE_UQ,
> + /** Integer types: @{ */
> BRW_REGISTER_TYPE_Q,
> + BRW_REGISTER_TYPE_UQ,
> + BRW_REGISTER_TYPE_D,
> + BRW_REGISTER_TYPE_UD,
> + BRW_REGISTER_TYPE_W,
> + BRW_REGISTER_TYPE_UW,
> + BRW_REGISTER_TYPE_B,
> + BRW_REGISTER_TYPE_UB,
> + BRW_REGISTER_TYPE_V,
> + BRW_REGISTER_TYPE_UV,
> + /** @} */
> };
>
> unsigned brw_reg_type_to_hw_type(const struct gen_device_info *devinfo,
> diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
> index 410922c62b..bf9a271900 100644
> --- a/src/intel/compiler/brw_vec4.cpp
> +++ b/src/intel/compiler/brw_vec4.cpp
> @@ -42,8 +42,8 @@ void
> src_reg::init()
> {
> memset(this, 0, sizeof(*this));
> -
> this->file = BAD_FILE;
> + this->type = BRW_REGISTER_TYPE_UD;
> }
>
> src_reg::src_reg(enum brw_reg_file file, int nr, const glsl_type *type)
> @@ -85,6 +85,7 @@ dst_reg::init()
> {
> memset(this, 0, sizeof(*this));
> this->file = BAD_FILE;
> + this->type = BRW_REGISTER_TYPE_UD;
> this->writemask = WRITEMASK_XYZW;
> }
>
> --
> 2.13.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
