[Mesa-dev] [PATCH] i965/fs: Replace subreg_offset with brw_reg's subnr.

[Francisco Jerez]

Kenneth Graunke <kenneth at whitecape.org> writes:

> On Thursday, February 11, 2016 5:49:55 PM PST Matt Turner wrote:
>> On Thu, Feb 11, 2016 at 3:33 PM, Francisco Jerez <currojerez at riseup.net> 
> wrote:
>> > Would be really nice if we could also get rid of reg_offset as we're at
>> > it.  reg and subreg_offset basically represent the same thing but with
>> > different units, couldn't we just have a single offset field in bytes?
>> > Should it be part of brw_reg or backend_reg?  I think I would lean
>> > towards backend_reg.  In that case does it make sense to move this into
>> > brw_reg now only to move it back to backend_reg later on?
>> 
>> That would be nice.
>>
>> I'm just not sure how to do it. brw_reg has to have the subnr field,
>> and it's nice if that's the field the higher levels use too.
>> 
>> I wonder -- is it possible that we could just get rid of reg_offset
>> too? For gathering data we have load_payload, so it's not useful
>> there. I think it's mainly useful for accessing elements of texturing
>> results. Is doubt there is a way we could avoid that though?
>
> I disagree - I don't think this would be nice at all.
>
> When we designed the IR, we needed something to handle cases like
> texturing results, where we actually need to store a whole vec4,
> and can't break it into separate scalar components.  (Note that
> messages used MRFs; we didn't even know about send-from-GRF.)
>
> To handle this, Eric created an abstraction of "virtual registers with
> size > 1", where we basically have an array of registers, each of which
> holds a single scalar value.  It can be thought of as a vecN, and
> reg_offset is the array index - v[i] - selecting each of the logically
> contiguous components.  reg_offset only makes sense on these large
> virtual registers - it has no meaning for real hardware registers.
>
> In contrast, subreg_offset (which came later) offered a way to access
> particular channels of a single hardware register, such as g0.2.  We
> added this later to make the IR more expressive.
>
> While these are both offsets, they serve very different purposes.
>
That hasn't been the case for a while, reg_offset is expressed in
multiples of a GRF, IOW it's just subreg_offset in different units,
there's no point in keeping both of them around, and it makes things
unnecessarily difficult and error-prone to have to take both of them
into account anytime you need to find out e.g. what region of a VGRF a
backend_reg refers to.  One of them needs to die IMO.

> Replacing subreg_offset with subnr makes a lot of sense to me, as
> both are basically a way to provide a byte offset for the start of
> a register region, allowing unaligned register access.  But reg_offset
> is a different beast.
>
> If you want to be rid of it, then perhaps we should consider removing
> the "VGRF of size > 1" abstraction.  One could imagine a system where
> we allocate separate VRFs for each scalar value, but record that
> "VRFs 4, 12, and 63 need to be contiguous", passing that information
> to the register allocator.  There are certainly other fine approaches.
>
I don't think I see what the benefit would be of replacing
multi-component VGRFs with such a thing.  Definitely a lot more work
than unifying reg_ and subreg_offset.

> I would also humbly request that you wait until FP64 lands before making
> any major changes.  It's very easy to conflate type size, number of SIMD
> channels, and register offsets, and a lot of the FP64 work is fixing
> places that are confused about that.  I'd really like to avoid making
> our Igalia friends' lives harder by making them rebase 100 patches on
> IR redesigns.
>
> --Ken
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160211/ad4d1b74/attachment.sig>