[Mesa-dev] [PATCH 6/6] gallivm: Add a new interface for doing TGSI->LLVM conversions
thomas.stellard at amd.com
Tue Jan 17 08:06:15 PST 2012
On Mon, 2012-01-16 at 11:27 -0800, Jose Fonseca wrote:
> 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.
> ----- 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
2. Add a few opcode actions for opcodes without default implementation
3. Adapt the emit_fetch_* functions so they can fetch scalar values
(like Temp.x). This is only for opcodes, that have scalar arguments,
and don't replicate their result or execute componentwise (e.g.
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
> > ---
> > 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.
More information about the mesa-dev