<div class="gmail_quote">On 20 September 2011 01:05, Tolga Dalman <span dir="ltr"><<a href="mailto:tolga.dalman@googlemail.com">tolga.dalman@googlemail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><div></div><div class="h5"><br>
</div></div>I wouldn't exactly call it 'optimisation' :) However, I feel a bit uneasy about using the == operator on float variables<br>
as this could lead to unexpected errors.<br>
<br>
Best regards<br>
<font color="#888888">Tolga Dalman<br></font></blockquote><div><br></div><div>Tolga, your uneasiness is certainly justified in terms of conventional wisdom. As someone who used to write numerical analysis software, I cringe whenever I see floats compared using ==, because I know that this is almost always a bad idea because of rounding errors. But there are occasional exceptions, and I think this is one of them, for two reasons:</div>
<div><br></div><div>(a) most of the time when one is tempted to compare two floats for equality, it's worse to have a false negative (classifying two floats as unequal when in fact they're only unequal because of rounding errors) than to have a false positive (classifying two floats as equal when in fact they're not quite equal because of rounding errors). But since the is_one() and is_zero() functions are used to identify opportunities for optimizing the shader, a false negative just means that we might occasionally generate less optimal code, and a false positive means the shader would produce incorrect outputs. In this case a false positive is worse, so we should err on the side of classifying floats as unequal, even if they are very close.</div>
<div><br></div><div>(b) the vast majority of the 1.0 and 0.0 values that we need to identify with the is_one() and is_zero() functions either come from literal 1.0 and 0.0 values (either in Mesa code or in the user's shaders) or from trivial algebraic simplifications like 2.0 - 1.0 => 1.0. Since these values can be represented perfectly in floats, there isn't an opportunity for rounding errors to accumulate.</div>
<div><br></div><div>I also found your proposed alternative a little surprising:</div><div><br></div><div><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; background-color: rgb(255, 255, 255); ">fabs(imm.f - 1.0) < std::numeric_limits<float>::epsilon()</span></div>
<div><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; background-color: rgb(255, 255, 255); "><br></span></div><div><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; background-color: rgb(255, 255, 255); ">When rounding errors accumulate, they typically wind up being much larger than epsilon(). If your goal is to make the test tolerant of rounding errors, wouldn't you need a value much larger than epsilon() as your threshold? Or is your goal to get the same computational effect without the cringe-worthiness of using "=="?</span></div>
<div><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; background-color: rgb(255, 255, 255); "><br></span></div><div><span class="Apple-style-span" style="font-family: arial, sans-serif; font-size: 13px; background-color: rgb(255, 255, 255); ">In any case I think this discussion may be moot, because AFAICT this patch doesn't apply on master--brw_vec4.cpp has never had a src_reg_is_zero() or src_reg_is_one() function. Or am I missing something?</span></div>
</div>