[Mesa-dev] [PATCH 00/24] i965: Refactor register classes

[Emil Velikov]

Hi Matt,

On 3 November 2015 at 00:29, Matt Turner <mattst88 at gmail.com> wrote:
> backend_reg (from which fs_reg, src_reg, and dst_reg inherit) includes a
> brw_reg that's used for "hardware regs" -- precolored registers or architecture
> registers. This leads to properties like source modifiers, the register type,
> swizzles, and writemasks being duplicated between the derived classes and the
> brw_reg and of course often being out of sync.
>
Great work ! While looking around the *_reg::init() I noticed a few
places where we don't derive (clone all the member variables) new
*_reg correctly. Plus I believe I've seen a few initialised member
variables - mostly on the abs/negate front.

I'm curious of there could attribute to the intermittent piglit
failures that people occasionally see.

> This series removes the "fixed_hw_reg" field from backend_reg by just making
> backend_reg inherit from brw_reg, and then removes fields duplicated in the
> derived classes. In the process, it gets rid of HW_REG.
>
I remember initially looking at fixed_hw_reg and having a few 'wtf'
moments. Inheriting from brw_reg will certainly help other (fs/vec4
IR) new-comers.

> This in turn simplifies a lot of code -- no longer do you have to check a
> number of subfields if file == HW_REG.
>
> The last few patches begin some clean ups -- since the base of our register
> classes is now brw_reg we don't need to do as many conversions. I've only
> handled immediates so far and more is planned, but the series is growing large
> and is a lot of churn already.
>
> The sizes of the register classes all shrink by 8 bytes:
>
>    backend_reg   20 -> 12
>    fs_reg        40 -> 32
>    src_reg       32 -> 24?
>    dst_reg       32 -> 24?
>
> The remaining fields in the classes are
>
>    backend_reg: reg_offset
>    fs_reg:      reladdr, subreg_offset, stride
>    src_reg      reladdr
>    dst_reg      reladdr
>
I'm thinking that we can also move reladdr to backend_reg. Although
that'll be obviously follow up patches.

Fwiw I really like the whole series and barring a few small comments I
would love to see it land.


Big thanks for doing this.

-Emil