[Mesa-dev] [PATCH v3 08/48] intel/eu: Just modify the offset in brw_broadcast

[Iago Toral]

I am not sure I get the purpose of this, there is nothing wrong with
the change, but the subject suggests that was so that we modified that
offset only inside brw_broadcast()... but that was already happening
and in fact this patch only changes code inside that function so I
wonder if there is something else missing or maybe the subject should
be changed.

Iago

On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote:
> This means we have to drop const from a variable but it also means
> that
> 100% of the code which deals with the offset limit is in one place.
> ---
>  src/intel/compiler/brw_eu_emit.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/src/intel/compiler/brw_eu_emit.c
> b/src/intel/compiler/brw_eu_emit.c
> index e10b143..a18cfa4 100644
> --- a/src/intel/compiler/brw_eu_emit.c
> +++ b/src/intel/compiler/brw_eu_emit.c
> @@ -3404,7 +3404,7 @@ brw_broadcast(struct brw_codegen *p,
>        if (align1) {
>           const struct brw_reg addr =
>              retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD);
> -         const unsigned offset = src.nr * REG_SIZE + src.subnr;
> +         unsigned offset = src.nr * REG_SIZE + src.subnr;
>           /* Limit in bytes of the signed indirect addressing
> immediate. */
>           const unsigned limit = 512;
>  
> @@ -3422,15 +3422,16 @@ brw_broadcast(struct brw_codegen *p,
>            * addressing immediate, account for the difference if the
> source
>            * register is above this limit.
>            */
> -         if (offset >= limit)
> +         if (offset >= limit) {
>              brw_ADD(p, addr, addr, brw_imm_ud(offset - offset %
> limit));
> +            offset = offset % limit;
> +         }
>  
>           brw_pop_insn_state(p);
>  
>           /* Use indirect addressing to fetch the specified
> component. */
>           brw_MOV(p, dst,
> -                 retype(brw_vec1_indirect(addr.subnr, offset %
> limit),
> -                        src.type));
> +                 retype(brw_vec1_indirect(addr.subnr, offset),
> src.type));
>        } else {
>           /* In SIMD4x2 mode the index can be either zero or one,
> replicate it
>            * to all bits of a flag register,