On 15 September 2012 21:25, Kenneth Graunke <kenn...@whitecape.org> wrote:
> In the past, we stored booleans as integer 0 or 1. At some point, we > changed to storing them as 0 or some non-zero value. Nit-pick: I'm assuming this means "0 is treated as false, and all nonzero values are treated as true". But that can't be right, since resolve_bool_comparison() computes (x & 1). Do we only use a subset of the possible nonzero values to represent true? (If so it would be nice to document that in resolve_bool_comparison()) Or is resolve_bool_comparison() wrong? In any case this change seems correct, so: Reviewed-by: Paul Berry <stereotype...@gmail.com> Note: I believe the case for ir_binop_logic_xor (in the same function) also needs fixing, since it treats "if (x ^ y)" as "if (x != y)". That won't work if x and y use different representations of true. > The Sandybridge > specific code for emitting IF instructions with embedded conditional > modifiers was not updated to account for this change. > > For code such as > > if (bool_a == bool_b) > > we cannot simply emit an equality comparison: even if both operands are > true, they may be represented by two different non-zero values. We must > first normalize them to 0 or 1 so that they can be compared properly. > > Fixes piglit test spec/glsl-1.10/execution/fs-bool-less-compare-true. > Fixes at least 9 subcases of oglconform's shad-compiler test: > - TestLessThanEquali > - TestGreaterThan > - TestGreaterThani > - TestGreaterThanEqual > - TestGreaterThanEquali > - TestEqualb > - TestNotEqual > - TestNotEquali > - TestNotEqualb > > NOTE: This is a candidate for the 9.0 branch (but not 8.0). > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54709 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=48629 > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > index d3cbde3..320e2f0 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp > @@ -1650,6 +1650,9 @@ fs_visitor::emit_if_gen6(ir_if *ir) > case ir_binop_all_equal: > case ir_binop_nequal: > case ir_binop_any_nequal: > + resolve_bool_comparison(expr->operands[0], &op[0]); > + resolve_bool_comparison(expr->operands[1], &op[1]); > + > inst = emit(BRW_OPCODE_IF, reg_null_d, op[0], op[1]); > inst->conditional_mod = > brw_conditional_for_comparison(expr->operation); > -- > 1.7.11.4 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
