On 16/07/15 17:33, Francisco Jerez wrote:
> When the width field was removed from fs_reg the BROADCAST handling
> code in opt_algebraic() started to miss a number of trivial
> optimization cases resulting in the ugly indirect-addressing sequence
> to be emitted unnecessarily for some variable-indexed texturing and
> UBO loads regardless of one of the sources of BROADCAST being
> immediate. Apparently the reason was that we were setting the stride
> field to one for immediates even though they are typically uniform.
> Width used to be set to one too which is why this optimization used to
> work previously until the "reg.width == 1" check was removed.
>
> The stride field of vector immediates is intentionally left equal to
> one, because they are strictly speaking not uniform. The assertion in
> fs_generator makes sure that immediates have the expected stride as
> consistency check.
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 3 +++
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 4 ++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index ff0675d..537ccbe 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -362,6 +362,7 @@ fs_reg::fs_reg(float f)
> init();
> this->file = IMM;
> this->type = BRW_REGISTER_TYPE_F;
> + this->stride = 0;
> this->fixed_hw_reg.dw1.f = f;
> }
>
> @@ -371,6 +372,7 @@ fs_reg::fs_reg(int32_t i)
> init();
> this->file = IMM;
> this->type = BRW_REGISTER_TYPE_D;
> + this->stride = 0;
> this->fixed_hw_reg.dw1.d = i;
> }
>
> @@ -380,6 +382,7 @@ fs_reg::fs_reg(uint32_t u)
> init();
> this->file = IMM;
> this->type = BRW_REGISTER_TYPE_UD;
> + this->stride = 0;
> this->fixed_hw_reg.dw1.ud = u;
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index bae7216..8a3af47 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -79,6 +79,10 @@ brw_reg_from_fs_reg(fs_inst *inst, fs_reg *reg)
> brw_reg = byte_offset(brw_reg, reg->subreg_offset);
> break;
> case IMM:
> + assert(reg->stride == (reg->type == BRW_REGISTER_TYPE_V ||
> + reg->type == BRW_REGISTER_TYPE_UV ||
> + reg->type == BRW_REGISTER_TYPE_VF ? 1 : 0));
> +
Just nitpicking: I would put extra parenthesis to enclose the whole
condition.
Reviewed-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
Sam
> switch (reg->type) {
> case BRW_REGISTER_TYPE_F:
> brw_reg = brw_imm_f(reg->fixed_hw_reg.dw1.f);
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
