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

[Bryan Cain]

On 08/27/2011 05:39 AM, Christoph Bumiller wrote:
> 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.

For now, I'm trying to get things working.  Once integers are working on
softpipe and r600g with no piglit regressions, I'll look into
optimizations like using AND.

>> +/**
>>   * 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.
>

That line of code emits USNE, not SNE.  See the get_opcode() function.

>> @@ -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);