[Mesa-dev] [PATCH] i965: Remove end-of-thread SEND alignment code.

[Kenneth Graunke]

On Tuesday, April 21, 2015 03:25:07 PM Matt Turner wrote:
> This was present in Eric's initial implementation of the compaction code
> for Sandybridge (commit 077d01b6). There is no documentation saying this
> is necessary, and removing it causes no regressions in piglit on any
> platform.
> 
> In fact, the BSpec says
> 
>    - "[Nop and Illegal] cannot be compressed."; and

That's "compression", not "compaction" - it doesn't seem related.
Compressed NOPs or ILLEGALs (nop(16) that decodes as two nop(8)s?) do
seem entirely pointless.  Compacted NOPs make sense.

>    - "Currently, there is no need for between-instruction padding."

This text appears in the original 965 PRM...and that platform didn't
support instruction compaction at all.  So I expect the text has been
carried forward from generation to generation without much thought,
rather than intended to convey something about compacted NOPs.

The fact that this hasn't broken anything is compelling, though...I
wonder if we had a bug in the code to create padding between SIMD8 and
SIMD16 programs, and aligning the EOT on the SIMD8 prevented the entire
SIMD16 program from getting misaligned?

I can't find any documentation indicating it's necessary either.

> ---
>  src/mesa/drivers/dri/i965/brw_eu_compact.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> index 26c41ea..72adbe0 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c
> @@ -1398,19 +1398,11 @@ brw_compact_instructions(struct brw_compile *p, int start_offset,
>  
>           offset += sizeof(brw_compact_inst);
>        } else {
> -         /* It appears that the end of thread SEND instruction needs to be
> -          * aligned, or the GPU hangs. All uncompacted instructions need to be
> -          * aligned on G45.
> -          */
> -         if ((offset & sizeof(brw_compact_inst)) != 0 &&
> -             (((brw_inst_opcode(brw, src) == BRW_OPCODE_SEND ||
> -                brw_inst_opcode(brw, src) == BRW_OPCODE_SENDC) &&
> -               brw_inst_eot(brw, src)) ||
> -              brw->is_g4x)) {
> +         /* All uncompacted instructions need to be aligned on G45. */
> +         if ((offset & sizeof(brw_compact_inst)) != 0 && brw->is_g4x){
>              brw_compact_inst *align = store + offset;
>              memset(align, 0, sizeof(*align));
> -            brw_compact_inst_set_opcode(align, brw->is_g4x ? BRW_OPCODE_NENOP :
> -                                                             BRW_OPCODE_NOP);
> +            brw_compact_inst_set_opcode(align, BRW_OPCODE_NENOP);
>              brw_compact_inst_set_cmpt_control(align, true);
>              offset += sizeof(brw_compact_inst);
>              compacted_count--;
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150421/3947303f/attachment.sig>