----- Original Message -----
> On 02/01/2013 12:39 PM, srol...@vmware.com wrote:
> > From: Roland Scheidegger<srol...@vmware.com>
> >
> > They are similar to old-style tex opcodes but with separate sampler
> > and
> > texture units (and other arguments in different places).
> > Also adjust the debug tgsi dump code.
> > ---
> > src/gallium/auxiliary/gallivm/lp_bld_tgsi.h | 6 +-
> > src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c | 115 ++++++++-
> > src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 291
> > ++++++++++++++++++++++
> > 3 files changed, 406 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h
> > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h
> > index 4898849..5d6b737 100644
> > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h
> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h
> > @@ -68,7 +68,8 @@ enum lp_build_tex_modifier {
> > LP_BLD_TEX_MODIFIER_PROJECTED,
> > LP_BLD_TEX_MODIFIER_LOD_BIAS,
> > LP_BLD_TEX_MODIFIER_EXPLICIT_LOD,
> > - LP_BLD_TEX_MODIFIER_EXPLICIT_DERIV
> > + LP_BLD_TEX_MODIFIER_EXPLICIT_DERIV,
> > + LP_BLD_TEX_MODIFIER_LZ
>
> Could you put a comment on the last one to say what LZ means?
>
>
> > };
> >
> >
> > @@ -104,7 +105,8 @@ struct lp_tgsi_texture_info
> > {
> > struct lp_tgsi_channel_info coord[4];
> > unsigned target:8; /* TGSI_TEXTURE_* */
> > - unsigned unit:8; /* Sampler unit */
> > + unsigned sampler_unit:8; /* Sampler unit */
> > + unsigned texture_unit:8; /* Texture unit */
> > unsigned modifier:8; /* LP_BLD_TEX_MODIFIER_* */
>
> Maybe we could just use ubyte instead of bitfields?
We could now, but as limits keep growing we'll want to stick with bitfields for
compactness.
> > };
> >
> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c
> > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c
> > index 1c54ef9..2a445d6 100644
> > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c
> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_info.c
> > @@ -140,14 +140,104 @@ analyse_tex(struct analysis_context *ctx,
> > if (modifier == LP_BLD_TEX_MODIFIER_EXPLICIT_DERIV) {
> > /* We don't track explicit derivatives, although we
> > could */
> > indirect = TRUE;
> > - tex_info->unit = inst->Src[3].Register.Index;
> > + tex_info->sampler_unit = inst->Src[3].Register.Index;
> > + tex_info->texture_unit = inst->Src[3].Register.Index;
> > } else {
> > if (modifier == LP_BLD_TEX_MODIFIER_PROJECTED ||
> > modifier == LP_BLD_TEX_MODIFIER_LOD_BIAS ||
> > modifier == LP_BLD_TEX_MODIFIER_EXPLICIT_LOD) {
> > readmask |= TGSI_WRITEMASK_W;
> > }
> > - tex_info->unit = inst->Src[1].Register.Index;
> > + tex_info->sampler_unit = inst->Src[1].Register.Index;
> > + tex_info->texture_unit = inst->Src[1].Register.Index;
> > + }
> > +
> > + for (chan = 0; chan< 4; ++chan) {
> > + struct lp_tgsi_channel_info *chan_info
> > =&tex_info->coord[chan];
> > + if (readmask& (1<< chan)) {
> > + analyse_src(ctx, chan_info,&inst->Src[0].Register,
> > chan);
> > + if (chan_info->file != TGSI_FILE_INPUT) {
> > + indirect = TRUE;
> > + }
> > + } else {
> > + memset(chan_info, 0, sizeof *chan_info);
> > + }
> > + }
> > +
> > + if (indirect) {
> > + info->indirect_textures = TRUE;
> > + }
> > +
> > + ++info->num_texs;
> > + } else {
> > + info->indirect_textures = TRUE;
> > + }
> > +}
> > +
> > +
> > +static void
> > +analyse_sample(struct analysis_context *ctx,
> > + const struct tgsi_full_instruction *inst,
> > + enum lp_build_tex_modifier modifier)
>
> It would be good to have a comment on this function (and
> analyse_tex()) to say something about what they do.
>
>
> > +{
> > + struct lp_tgsi_info *info = ctx->info;
> > + unsigned chan;
> > +
> > + if (info->num_texs< Elements(info->tex)) {
> > + struct lp_tgsi_texture_info *tex_info
> > =&info->tex[info->num_texs];
> > + boolean indirect = FALSE;
> > + boolean shadow = FALSE;
> > + unsigned readmask = 0;
> > +
> > + tex_info->target = inst->Texture.Texture;
> > + switch (inst->Texture.Texture) {
> > + case TGSI_TEXTURE_SHADOW1D:
> > + shadow = TRUE;
> > + /* Fallthrough */
> > + case TGSI_TEXTURE_1D:
> > + readmask = TGSI_WRITEMASK_X;
> > + break;
> > + case TGSI_TEXTURE_SHADOW1D_ARRAY:
> > + case TGSI_TEXTURE_SHADOW2D:
> > + case TGSI_TEXTURE_SHADOWRECT:
> > + shadow = TRUE;
> > + /* Fallthrough */
> > + case TGSI_TEXTURE_1D_ARRAY:
> > + case TGSI_TEXTURE_2D:
> > + case TGSI_TEXTURE_RECT:
> > + readmask = TGSI_WRITEMASK_XY;
> > + break;
> > + case TGSI_TEXTURE_SHADOW2D_ARRAY:
> > + case TGSI_TEXTURE_SHADOWCUBE:
> > + shadow = TRUE;
> > + /* Fallthrough */
> > + case TGSI_TEXTURE_2D_ARRAY:
> > + case TGSI_TEXTURE_3D:
> > + case TGSI_TEXTURE_CUBE:
> > + readmask = TGSI_WRITEMASK_XYZ;
> > + break;
> > + case TGSI_TEXTURE_SHADOWCUBE_ARRAY:
> > + shadow = TRUE;
> > + /* Fallthrough */
> > + case TGSI_TEXTURE_CUBE_ARRAY:
> > + readmask = TGSI_WRITEMASK_XYZW;
> > + break;
> > + default:
> > + assert(0);
> > + return;
> > + }
I wonder if we could share more code between analyse_tex/sample and
emit_tex/sample?
Other than this and Brian's comments also looks good to me.
Jose
> > +
> > + tex_info->texture_unit = inst->Src[1].Register.Index;
> > + tex_info->sampler_unit = inst->Src[2].Register.Index;
> > +
> > + if (modifier == LP_BLD_TEX_MODIFIER_EXPLICIT_DERIV ||
> > + modifier == LP_BLD_TEX_MODIFIER_LOD_BIAS || shadow) {
> > + /* We don't track insts with additional regs, although we
> > could */
> > + indirect = TRUE;
> > + } else {
> > + if (modifier == LP_BLD_TEX_MODIFIER_EXPLICIT_LOD) {
> > + readmask |= TGSI_WRITEMASK_W;
> > + }
> > }
> >
> > for (chan = 0; chan< 4; ++chan) {
> > @@ -229,6 +319,22 @@ analyse_instruction(struct analysis_context
> > *ctx,
> > case TGSI_OPCODE_TXP:
> > analyse_tex(ctx, inst, LP_BLD_TEX_MODIFIER_PROJECTED);
> > break;
> > + case TGSI_OPCODE_SAMPLE:
> > + case TGSI_OPCODE_SAMPLE_C:
> > + analyse_sample(ctx, inst, LP_BLD_TEX_MODIFIER_NONE);
> > + break;
> > + case TGSI_OPCODE_SAMPLE_C_LZ:
> > + analyse_sample(ctx, inst, LP_BLD_TEX_MODIFIER_LZ);
> > + break;
> > + case TGSI_OPCODE_SAMPLE_D:
> > + analyse_sample(ctx, inst,
> > LP_BLD_TEX_MODIFIER_EXPLICIT_DERIV);
> > + break;
> > + case TGSI_OPCODE_SAMPLE_B:
> > + analyse_sample(ctx, inst, LP_BLD_TEX_MODIFIER_LOD_BIAS);
> > + break;
> > + case TGSI_OPCODE_SAMPLE_L:
> > + analyse_sample(ctx, inst,
> > LP_BLD_TEX_MODIFIER_EXPLICIT_LOD);
> > + break;
> > default:
> > break;
> > }
> > @@ -355,8 +461,9 @@ dump_info(const struct tgsi_token *tokens,
> > debug_printf(" _");
> > }
> > }
> > - debug_printf(", SAMP[%u], %s\n",
> > - tex_info->unit,
> > + debug_printf(", RES[%u], SAMP[%u], %s\n",
> > + tex_info->texture_unit,
> > + tex_info->sampler_unit,
> > tgsi_texture_names[tex_info->target]);
> > }
> >
> > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > index 0621fb4..c6801ac 100644
> > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> > @@ -1340,6 +1340,197 @@ emit_tex( struct lp_build_tgsi_soa_context
> > *bld,
> > }
> >
> > static void
> > +emit_sample(struct lp_build_tgsi_soa_context *bld,
> > + const struct tgsi_full_instruction *inst,
> > + enum lp_build_tex_modifier modifier,
> > + LLVMValueRef *texel)
> > +{
> > + LLVMBuilderRef builder = bld->bld_base.base.gallivm->builder;
> > + struct gallivm_state *gallivm = bld->bld_base.base.gallivm;
> > + unsigned texture_unit, sampler_unit;
> > + LLVMValueRef lod_bias, explicit_lod;
> > + LLVMValueRef coords[4];
> > + LLVMValueRef offsets[3] = { NULL };
> > + struct lp_derivatives derivs;
> > + unsigned num_coords, dims;
> > + unsigned i;
> > + boolean compare = FALSE;
> > +
> > + if (!bld->sampler) {
> > + _debug_printf("warning: found texture instruction but no
> > sampler generator supplied\n");
> > + for (i = 0; i< 4; i++) {
> > + texel[i] = bld->bld_base.base.undef;
> > + }
> > + return;
> > + }
> > +
> > + derivs.ddx_ddy[0] = bld->bld_base.base.undef;
> > + derivs.ddx_ddy[1] = bld->bld_base.base.undef;
> > +
> > + switch (inst->Texture.Texture) {
> > + case TGSI_TEXTURE_SHADOW1D:
> > + compare = TRUE;
> > + /* Fallthrough */
> > + case TGSI_TEXTURE_1D:
> > + num_coords = 1;
> > + dims = 1;
> > + break;
> > + case TGSI_TEXTURE_SHADOW1D_ARRAY:
> > + compare = TRUE;
> > + /* Fallthrough */
> > + case TGSI_TEXTURE_1D_ARRAY:
> > + num_coords = 2;
> > + dims = 1;
> > + break;
> > + case TGSI_TEXTURE_SHADOW2D:
> > + case TGSI_TEXTURE_SHADOWRECT:
> > + compare = TRUE;
> > + /* Fallthrough */
> > + case TGSI_TEXTURE_2D:
> > + case TGSI_TEXTURE_RECT:
> > + num_coords = 2;
> > + dims = 2;
> > + break;
> > + case TGSI_TEXTURE_SHADOW2D_ARRAY:
> > + case TGSI_TEXTURE_SHADOWCUBE:
> > + compare = TRUE;
> > + /* Fallthrough */
> > + case TGSI_TEXTURE_2D_ARRAY:
> > + case TGSI_TEXTURE_CUBE:
> > + num_coords = 3;
> > + dims = 2;
> > + break;
> > + case TGSI_TEXTURE_3D:
> > + num_coords = 3;
> > + dims = 3;
> > + break;
> > + case TGSI_TEXTURE_SHADOWCUBE_ARRAY:
> > + compare = TRUE;
> > + /* Fallthrough */
> > + case TGSI_TEXTURE_CUBE_ARRAY:
> > + num_coords = 4;
> > + dims = 3;
> > + break;
> > + default:
> > + assert(0);
> > + return;
> > + }
> > +
> > + /* unlike tex opcodes the sampler/texture unit always come from
> > same reg */
> > + texture_unit = inst->Src[1].Register.Index;
> > + sampler_unit = inst->Src[2].Register.Index;
>
> The comment and code don't seem to agree there.
>
> [...]
>
>
> Otherwise looks good AFAICT.
>
> Reviewed-by: Brian Paul <bri...@vmware.com>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
