[Mesa-dev] [PATCH] mesa, glsl_to_tgsi: fixes for native integers

[Christoph Bumiller]

On 27.08.2011 04:58, Bryan Cain wrote:
> This fixes all of the piglit regressions in softpipe when native integers are
> enabled.
> ---
>  src/mesa/main/uniforms.c                   |    8 +----
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp |   45 ++++++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
> index cda840f..0801476 100644
> --- a/src/mesa/main/uniforms.c
> +++ b/src/mesa/main/uniforms.c
> @@ -776,13 +776,7 @@ set_program_uniform(struct gl_context *ctx, struct gl_program *program,
>           /* if the uniform is bool-valued, convert to 1 or 0 */
>           if (isUniformBool) {
>              for (i = 0; i < elems; i++) {
> -               if (basicType == GL_FLOAT)
> -                  uniformVal[i].b = uniformVal[i].f != 0.0f ? 1 : 0;
> -               else
> -                  uniformVal[i].b = uniformVal[i].u ? 1 : 0;
> -               
> -               if (!ctx->Const.NativeIntegers)
> -                  uniformVal[i].f = uniformVal[i].b ? 1.0f : 0.0f;
> +               uniformVal[i].f = uniformVal[i].f != 0.0f ? 1.0f : 0.0f;
>              }
>           }
>        }
> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> index 9cac309..d1674eb 100644
> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> @@ -385,6 +385,8 @@ public:
>     void emit_scalar(ir_instruction *ir, unsigned op,
>          	    st_dst_reg dst, st_src_reg src0, st_src_reg src1);
>  
> +   void try_emit_integer_set(ir_instruction *ir, unsigned op, st_dst_reg dst);
> +
>     void emit_arl(ir_instruction *ir, st_dst_reg dst, st_src_reg src0);
>  
>     void emit_scs(ir_instruction *ir, unsigned op,
> @@ -563,6 +565,8 @@ glsl_to_tgsi_visitor::emit(ir_instruction *ir, unsigned op,
>  
>     this->instructions.push_tail(inst);
>     
> +   try_emit_integer_set(ir, op, dst);
> +   
>     return inst;
>  }
>  
> @@ -589,6 +593,32 @@ glsl_to_tgsi_visitor::emit(ir_instruction *ir, unsigned op)
>  }
>  
>  /**
> + * Emits the code to convert the result of integer SET instructions to floats.
> + */
> +void
> +glsl_to_tgsi_visitor::try_emit_integer_set(ir_instruction *ir, unsigned op,
> +        		 st_dst_reg dst)
> +{
> +   st_src_reg src;
> +
> +   if (!(op == TGSI_OPCODE_USEQ ||
> +         op == TGSI_OPCODE_USNE ||
> +         op == TGSI_OPCODE_ISGE ||
> +         op == TGSI_OPCODE_USGE ||
> +         op == TGSI_OPCODE_ISLT ||
> +         op == TGSI_OPCODE_USLT))
> +	   return;
> +
> +   src = st_src_reg(dst);
> +   src.type = GLSL_TYPE_INT;
> +
> +   dst.type = GLSL_TYPE_FLOAT;
> +   emit(ir, TGSI_OPCODE_I2F, dst, src);
> +   src.type = GLSL_TYPE_FLOAT;
> +   emit(ir, TGSI_OPCODE_SNE, dst, src, st_src_reg_for_float(0.0));
> +}
> +

emit(ir, TGSI_OPCODE_AND, dst, src, st_src_reg_for_float(1.0f));

I still don't quite like booleans as floats, but I guess I can just
easily track back to the source SET to emit my set-predicate-register op
without having all the fuss in between.

> +/**
>   * Determines whether to use an integer, unsigned integer, or float opcode 
>   * based on the operands and input opcode, then emits the result.
>   * 
> @@ -1662,7 +1692,6 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir)
>        emit_scalar(ir, TGSI_OPCODE_RSQ, result_dst, op[0]);
>        break;
>     case ir_unop_i2f:
> -   case ir_unop_b2f:
>        if (native_integers) {
>           emit(ir, TGSI_OPCODE_I2F, result_dst, op[0]);
>           break;
> @@ -1670,10 +1699,14 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir)
>     case ir_unop_i2u:
>     case ir_unop_u2i:
>        /* Converting between signed and unsigned integers is a no-op. */
> -   case ir_unop_b2i:
> -      /* Booleans are stored as integers (or floats in GLSL 1.20 and lower). */
> +   case ir_unop_b2f:
> +      /* Booleans are stored as floats. */
>        result_src = op[0];
>        break;
> +   case ir_unop_b2i:
> +      if (native_integers)
> +         emit(ir, TGSI_OPCODE_F2I, result_dst, op[0]);
> +      break;
>     case ir_unop_f2i:
>        if (native_integers)
>           emit(ir, TGSI_OPCODE_F2I, result_dst, op[0]);
> @@ -1681,6 +1714,11 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir)
>           emit(ir, TGSI_OPCODE_TRUNC, result_dst, op[0]);
>        break;
>     case ir_unop_f2b:
> +      if (native_integers) {
> +         emit(ir, TGSI_OPCODE_SNE, result_dst, op[0], st_src_reg_for_float(0.0));
> +         emit(ir, TGSI_OPCODE_F2I, result_dst, result_src);

This is inconsistent, why do you convert to integer here if you store
all booleans as floats ?

> +      }
> +      break;
>     case ir_unop_i2b:
>        emit(ir, TGSI_OPCODE_SNE, result_dst, op[0], 
>              st_src_reg_for_type(result_dst.type, 0));

This can go wrong if the integer has the value of NaN and the
hardware/backend does not use an unordered comparison.

nv50 is using unordered, x86/gcc also uses >u<comiss by default, r600
docs don't say what the result of SETNE with NaN operand is so good luck.

> @@ -2154,6 +2192,7 @@ glsl_to_tgsi_visitor::visit(ir_assignment *ir)
>        inst = (glsl_to_tgsi_instruction *)this->instructions.get_tail();
>        new_inst = emit(ir, inst->op, l, inst->src[0], inst->src[1], inst->src[2]);
>        new_inst->saturate = inst->saturate;
> +      inst->dead_mask = inst->dst.writemask;
>     } else {
>        for (i = 0; i < type_size(ir->lhs->type); i++) {
>           emit(ir, TGSI_OPCODE_MOV, l, r);