-----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----- _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev