[Mesa-dev] [PATCH 11/19] i965: Convert brw_eu_compact.c to the new brw_inst API.
mattst88 at gmail.com
Sat Jun 14 17:41:37 PDT 2014
On Sat, Jun 14, 2014 at 12:54 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
>> - uint16_t compacted, uncompacted = 0;
>> - uncompacted |= (src->bits2.ud >> 13) & 0xfff;
>> + uint16_t compacted;
>> + uint16_t uncompacted = /* 12b */
>> + (brw_inst_src0_vstride(brw, src) << 8) | /* 4b */
>> + (brw_inst_src0_width(brw, src) << 5) | /* 3b */
>> + (brw_inst_src0_hstride(brw, src) << 3) | /* 2b */
> One thing that's a little funny here...we pull out hstride/width/vstride,
> which makes sense for align1 mode...but presumably this function is also used
> for align16 mode, where we instead have src0_da16_swiz_x etc.
> But, it's the same bits, so this ought to work. I'm not objecting, it's
> just...a little funny at first glance.
> I don't think it's worth adding conditionals, but would it be worth adding a
> comment saying basically /* this also works for align16 mode because they
> share the same bits */?
> Whatever you decide is fine. This patch is:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
I'm glad you mentioned this. I think I want to modify these to use
brw_inst_bits/brw_inst_set_bits instead. I know they're different on
Broadwell, so we'll have slightly more C, but I think the benefits
The SandyBridge docs give only the bitfields, while the IVB/HSW docs
give the bitfields and a useless list of out of order field names
(that regularly alias each other). So you have to rely on the bitfield
definitions anyway to make sense of it.
Also, the fields corresponding to the indicies are typically
contiguous. For instance, 16 of the 17 bits of SNB's control index
table are consecutive, so using brw_inst_bits lets us grab those
directly, without confusing the compiler into generating garbage by
extracting them separately, shifting, and oring them back together.
text data bss dec hex filename
4118 0 32 4150 1036 brw_eu_compact.o before conversion
6774 0 32 6806 1a96 brw_eu_compact.o after conversion
4662 0 32 4694 1256 brw_eu_compact.o after bitfield patch
There's evidently some overhead in the new approach. I've got an idea
how to fix it that I'm not sure is worth implementing, but I think
using the bit getter and setter is definitely the right way to go
More information about the mesa-dev