[Mesa-dev] [PATCH] gallium: remove support for predicates from TGSI

[Roland Scheidegger]

[resend with snipped bits as it's too big]

A couple comments inline.

[snip]

> --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c
> @@ -746,39 +746,30 @@ static void lp_exec_default(struct lp_exec_mask *mask,
>  }
>  
>  
>  /* stores val into an address pointed to by dst_ptr.
>   * mask->exec_mask is used to figure out which bits of val
>   * should be stored into the address
>   * (0 means don't store this bit, 1 means do store).
>   */
>  static void lp_exec_mask_store(struct lp_exec_mask *mask,
>                                 struct lp_build_context *bld_store,
> -                               LLVMValueRef pred,
>                                 LLVMValueRef val,
>                                 LLVMValueRef dst_ptr)
>  {
>     LLVMBuilderRef builder = mask->bld->gallivm->builder;
> +   LLVMValueRef pred = mask->has_mask ? mask->exec_mask : NULL;
Calling this "pred" now seems to be somewhat of a misnomer (wasn't all
that great before because it then included exec_mask but it's worse now).



>  
>     assert(lp_check_value(bld_store->type, val));
>     assert(LLVMGetTypeKind(LLVMTypeOf(dst_ptr)) == LLVMPointerTypeKind);
>     assert(LLVMGetElementType(LLVMTypeOf(dst_ptr)) == LLVMTypeOf(val));
>  
> -   /* Mix the predicate and execution mask */
> -   if (mask->has_mask) {
> -      if (pred) {
> -         pred = LLVMBuildAnd(builder, pred, mask->exec_mask, "");
> -      } else {
> -         pred = mask->exec_mask;
> -      }
> -   }
> -
>     if (pred) {
>        LLVMValueRef res, dst;
>  
>        dst = LLVMBuildLoad(builder, dst_ptr, "");
>        res = lp_build_select(bld_store, pred, val, dst);
>        LLVMBuildStore(builder, res, dst_ptr);
>     } else
>        LLVMBuildStore(builder, val, dst_ptr);
>  }
>  
> @@ -1029,36 +1020,26 @@ build_gather(struct lp_build_tgsi_context *bld_base,
>  
>  
>  /**
>   * Scatter/store vector.
>   */
>  static void
>  emit_mask_scatter(struct lp_build_tgsi_soa_context *bld,
>                    LLVMValueRef base_ptr,
>                    LLVMValueRef indexes,
>                    LLVMValueRef values,
> -                  struct lp_exec_mask *mask,
> -                  LLVMValueRef pred)
> +                  struct lp_exec_mask *mask)
>  {
>     struct gallivm_state *gallivm = bld->bld_base.base.gallivm;
>     LLVMBuilderRef builder = gallivm->builder;
>     unsigned i;
> -
> -   /* Mix the predicate and execution mask */
> -   if (mask->has_mask) {
> -      if (pred) {
> -         pred = LLVMBuildAnd(builder, pred, mask->exec_mask, "");
> -      }
> -      else {
> -         pred = mask->exec_mask;
> -      }
> -   }
> +   LLVMValueRef pred = mask->has_mask ? mask->exec_mask : NULL;
same here.


> diff --git a/src/gallium/include/pipe/p_shader_tokens.h b/src/gallium/include/pipe/p_shader_tokens.h
> index 6a3fb98..87d2d92 100644
> --- a/src/gallium/include/pipe/p_shader_tokens.h
> +++ b/src/gallium/include/pipe/p_shader_tokens.h
> @@ -62,21 +62,20 @@ struct tgsi_token
>  
>  enum tgsi_file_type {
>     TGSI_FILE_NULL,
>     TGSI_FILE_CONSTANT,
>     TGSI_FILE_INPUT,
>     TGSI_FILE_OUTPUT,
>     TGSI_FILE_TEMPORARY,
>     TGSI_FILE_SAMPLER,
>     TGSI_FILE_ADDRESS,
>     TGSI_FILE_IMMEDIATE,
> -   TGSI_FILE_PREDICATE,
>     TGSI_FILE_SYSTEM_VALUE,
>     TGSI_FILE_IMAGE,
>     TGSI_FILE_SAMPLER_VIEW,
>     TGSI_FILE_BUFFER,
>     TGSI_FILE_MEMORY,
>     TGSI_FILE_COUNT,      /**< how many TGSI_FILE_ types */
>  };
>  
>  
>  #define TGSI_WRITEMASK_NONE     0x00
> @@ -609,34 +608,31 @@ struct tgsi_property_data {
>  
>  /**
>   * Opcode is the operation code to execute. A given operation defines the
>   * semantics how the source registers (if any) are interpreted and what is
>   * written to the destination registers (if any) as a result of execution.
>   *
>   * NumDstRegs and NumSrcRegs is the number of destination and source registers,
>   * respectively. For a given operation code, those numbers are fixed and are
>   * present here only for convenience.
>   *
> - * If Predicate is TRUE, tgsi_instruction_predicate token immediately follows.
> - *
>   * Saturate controls how are final results in destination registers modified.
>   */
>  
>  struct tgsi_instruction
>  {
>     unsigned Type       : 4;  /* TGSI_TOKEN_TYPE_INSTRUCTION */
>     unsigned NrTokens   : 8;  /* UINT */
>     unsigned Opcode     : 8;  /* TGSI_OPCODE_ */
>     unsigned Saturate   : 1;  /* BOOL */
>     unsigned NumDstRegs : 2;  /* UINT */
>     unsigned NumSrcRegs : 4;  /* UINT */
> -   unsigned Predicate  : 1;  /* BOOL */
>     unsigned Label      : 1;
>     unsigned Texture    : 1;
>     unsigned Memory     : 1;
>     unsigned Padding    : 1;
The Padding doesn't match.



So, we still have code which uses this - however this code is only used
for some testing, otherwise we translate this d3d9 stuff away like
everybody else.
Maybe it's time to ditch this stuff then - clearly no other drivers are
ever going to support it and apis have moved away from it.

Jose, any opinion on that?

Roland