[Mesa-dev] [PATCH 6/6] gallivm: Add a new interface for doing TGSI->LLVM conversions

[Tom Stellard]

On Mon, 2012-01-16 at 11:27 -0800, Jose Fonseca wrote:
> Tom,
> 
> Looks good in principle!
> 
> But I need to test this myself before I can be comfortable w/ merging it into master.
> 
> Just a few quick comments inline.
> 
> Jose
> 
> ----- Original Message -----
> > From: Tom Stellard <thomas.stellard at amd.com>
> > 
> > lp_bld_tgsi_soa.c has been adapted to use this new interface, but
> > lp_bld_tgsi_aos.c has only been partially adapted, since nothing in
> > gallium currently uses it.
> 
> There are some closed source users of lp_bld_tgsi_aos.c. The rationale for open source it was to enable eventually using AoS in draw_llvm (as vertex shaders are usually more suitable for AoS), but it never happened yet.
> 
> Will lp_bld_tgsi_aos.c run correctly with this?
> 
> If it is not useful for r600g driver, then it might be better to leave lp_bld_tgsi_aos.c untouched for the time being. (I can do the conversion when I merge this into our private repositories).
> 

It should work with lp_bld_tgsi_aos too.  There's really not much work
still needed to convert the existing lp_bld_tgsi_aos code over to the
new interface.  Basically what needs to be done is:

1. Replace most of the remaining code with a call to
lp_build_tgsi_llvm().

2. Add a few opcode actions for opcodes without default implementation
(like TGSI_OPCODE_TEX).

3. Adapt the emit_fetch_* functions so they can fetch scalar values
(like Temp[0].x).  This is only for opcodes, that have scalar arguments,
and don't replicate their result or execute componentwise (e.g.
TGSI_OPOCDE_LOG).

I don't need lp_bld_tgsi_aos.c for r600g, so if you don't want changes
to lp_bld_tgsi_aos.c going into master, that's fine.  It's really up to
you, but please note that if lp_bld_tgsi_aos.c remains unchanged, we
will have to drop it from the Makefiles, because it won't build with the
new interface.


> > ---
> >  src/gallium/auxiliary/Makefile.sources          |    2 +
> >  src/gallium/auxiliary/gallivm/lp_bld_action.c   | 1182
> >  ++++++++++++++
> >  src/gallium/auxiliary/gallivm/lp_bld_action.h   |  138 ++
> >  src/gallium/auxiliary/gallivm/lp_bld_tgsi.c     |  409 +++++
> >  src/gallium/auxiliary/gallivm/lp_bld_tgsi.h     |  341 ++++-
> >  src/gallium/auxiliary/gallivm/lp_bld_tgsi_aos.c |  551 +++----
> >  src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 1981
> >  ++++++++---------------
> >  7 files changed, 2952 insertions(+), 1652 deletions(-)
> >  create mode 100644 src/gallium/auxiliary/gallivm/lp_bld_action.c
> >  create mode 100644 src/gallium/auxiliary/gallivm/lp_bld_action.h
> >  create mode 100644 src/gallium/auxiliary/gallivm/lp_bld_tgsi.c
> > 
> > diff --git a/src/gallium/auxiliary/Makefile.sources
> > b/src/gallium/auxiliary/Makefile.sources
> > index f55a4eb..547f63d 100644
> > --- a/src/gallium/auxiliary/Makefile.sources
> > +++ b/src/gallium/auxiliary/Makefile.sources
> > @@ -155,6 +155,7 @@ GENERATED_SOURCES := \
> >  	util/u_half.c
> >  
> >  GALLIVM_SOURCES := \
> > +        gallivm/lp_bld_action.c \
> >          gallivm/lp_bld_arit.c \
> >          gallivm/lp_bld_assert.c \
> >          gallivm/lp_bld_bitarit.c \
> > @@ -176,6 +177,7 @@ GALLIVM_SOURCES := \
> >          gallivm/lp_bld_sample_soa.c \
> >          gallivm/lp_bld_struct.c \
> >          gallivm/lp_bld_swizzle.c \
> > +	gallivm/lp_bld_tgsi.c \
> >          gallivm/lp_bld_tgsi_aos.c \
> >          gallivm/lp_bld_tgsi_info.c \
> >          gallivm/lp_bld_tgsi_soa.c \
> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_action.c
> > b/src/gallium/auxiliary/gallivm/lp_bld_action.c
> > new file mode 100644
> > index 0000000..0b6cc77
> > --- /dev/null
> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_action.c
> 
> I'd prefer a source name that clearly states it is tgsi related. For example lp_bld_tgsi_soa_action.c 
> 

Ok, I'll rename it to lp_bld_tgsi_action.c  Should I rename struct
lp_build_opcode_action, and struct lp_build_emit_data as well?


I didn't see any more comments, so I'm trimming the rest of the message,
so it doesn't get stuck in moderation again.

Thanks,
Tom