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 <[email protected]> > > > > 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 _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
