[Mesa-dev] [PATCH] mesa, glsl_to_tgsi: fixes for native integers
Ian Romanick
idr at freedesktop.org
Mon Aug 29 10:22:03 PDT 2011
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 08/27/2011 08:44 AM, Bryan Cain wrote:
> 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.
I think this is the underlying problem. Without this patch series, some
of the code treats booleans as integers, and some of it still tries to
apply floating-point tricks. Instead of reverting back to using
floating-point for booleans, it seems much better (by all metrics) to
finish the conversion to integer. Emitting (1-x) for logical-not or
(a*b) for logical-and when you have an actual NOT and AND instructions
is just silly.
Fixing that will make it irrelevant whether the integer set-on
instructions generate 1 or ~0 for true. Right?
>>> +/**
>>> * 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);
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk5byroACgkQX1gOwKyEAw/8YQCeOeRRDoO7hPxLLXnj0W8AX1U+
vQIAoInf7s1o4jnyjG3WgZ/hAzyrgrTp
=Q0GZ
-----END PGP SIGNATURE-----
More information about the mesa-dev
mailing list