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

[Matt Turner]

On Tue, Apr 21, 2015 at 10:08 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> 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.

Right, that's the millionth time someone has made that mistake :)

> 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.

Yeah, that's probably true. I'll drop both of these quotes.

> 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 think that seems like the most plausible reason. After my commit
that stopped passing over the SIMD8 program when compacting the SIMD16
program I bet if that was the problem stopped existing.

commit 9976294e867785ea480f52178a3d3dc67ac72d32
Author: Matt Turner <mattst88 at gmail.com>
Date:   Thu May 15 16:56:13 2014 -0700

    i965: Pass in start_offset to brw_compact_instructions().

    Let's us avoid recompacting the SIMD8 instructions when we compact the
    SIMD16 program.

    Reviewed-by: Eric Anholt <eric at anholt.net>

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

Yeah, that was Paul's assessment when he reviewed the original patch as well.

Reviewed-by/Acked-by?