[Mesa-dev] [PATCH 2/4] gallivm: Allow indirection from TEMP registers too.

[Jose Fonseca]

----- Original Message -----
> On 12/06/2012 10:35 AM, jfonseca at vmware.com wrote:
> > From: José Fonseca<jfonseca at vmware.com>
> >
> > The ADDR file is cumbersome for native integer capable drivers.  We
> > should consider deprecating it eventually, but this just adds
> > support
> > for indirection from TEMP registers.
> > ---
> >   src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c |   20
> >   +++++++++++++++++---
> >   1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > index 9caac21..bd783b5 100644
> > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > @@ -533,9 +533,23 @@ get_indirect_index(struct
> > lp_build_tgsi_soa_context *bld,
> >      base = lp_build_const_int_vec(bld->bld_base.base.gallivm,
> >      uint_bld->type, reg_index);
> >
> >      assert(swizzle<  4);
> > -   rel = LLVMBuildLoad(builder,
> > -                        bld->addr[indirect_reg->Index][swizzle],
> > -                        "load addr reg");
> > +   switch (indirect_reg->File) {
> > +   case TGSI_FILE_ADDRESS:
> > +      rel = LLVMBuildLoad(builder,
> > +                          bld->addr[indirect_reg->Index][swizzle],
> > +                          "load addr reg");
> > +      /* ADDR registers are uint */
> > +      break;
> > +   case TGSI_FILE_TEMPORARY:
> > +      rel = lp_get_temp_ptr_soa(bld, indirect_reg->Index,
> > swizzle);
> > +      rel = LLVMBuildLoad(builder, rel, "load temp reg");
> > +      /* TEMP registers are float type */
> > +      rel = LLVMBuildBitCast(builder, rel, uint_bld->vec_type,
> > "");
> 
> Is bitcasting right?  I'd expect that if the temp register has float
> values that we need to do a float->int conversion here.

The comment is not precise enough: LLVM is strongly typed, and TEMPS always have float LLVM type, but they actual value can be FLOAT/INT/UINT depending one the opcode, and for indirection, the index is expected to be an integer.

I'll update the comment with this.

> 
> > +      break;
> > +   default:
> > +      assert(0);
> > +      rel = uint_bld->zero;
> > +   }
> >
> >      index = lp_build_add(uint_bld, base, rel);
> >
> 
> Looks OK otherwise.
> 
> Reviewed-by: Brian Paul <brianp at vmware.com>
>