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

Iago Toral itoral at igalia.com
Fri Feb 12 10:15:47 UTC 2016


On Thu, 2016-02-11 at 19:12 -0800, Kenneth Graunke wrote:
> 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.
> 
> 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 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.

Thanks Ken for thinking about us :) I would definitely appreciate this!
The patch count for fp64 is already above 150 and rebasing it on top of
changes like these sounds quite scary to me.

Iago



More information about the mesa-dev mailing list