[Mesa-dev] [v2 PATCH 09/16] glsl: Optimize clamp(x, 0, 1) as saturate(x)

[Roland Scheidegger]

Am 08.07.2014 18:03, schrieb Erik Faye-Lund:
> On Tue, Jul 8, 2014 at 5:44 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>> Am 08.07.2014 08:22, schrieb Matt Turner:
>>> On Mon, Jul 7, 2014 at 11:00 PM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
>>>> On Mon, Jul 7, 2014 at 7:18 PM, Matt Turner <mattst88 at gmail.com> wrote:
>>>>> This comment tripped me up for a second. This really means that you've
>>>>> found either
>>>>>
>>>>>   - min(max(x, 0.0), 1.0); or
>>>>>   - max(min(x, 1.0), 0.0)
>>>>
>>>> Hmm, but are optimizing both of these to saturate OK? Shouldn't
>>>> min(max(NaN, 0.0), 1.0) give 0.0, whereas max(min(NaN, 1.0), 0.0) give
>>>> 1.0?
>>>
>>> Under standard IEEE rules, wouldn't the NaN propagate through all of
>>> these expressions?
>> not under ieee754-2008 rules, no. min/max return the other number if one
>> of them is a NaN.
> 
> I'm not entirely sure what IEEE754-2008 has to say about min/max, as
> GLSL already defines those operations (and states that IEEE754
> compliance isn't required). I was referring to the definition of the
> less-than comparison-operator, which GLSL seems to lean on. And that
> seems to always evaluate to 'false'.
Oh right I didn't notice min/max are defined that way. I guess that's
due to older specs - pre glsl 4 NaN support wasn't really required and
just about anything involving NaNs or Infs was ok to give undefined
results anyway (and some hw indeed didn't have NaN support).
Even current glsl version is not very strict there, still saying things
like "NaNs are not required to be generated" and "Operations and
built-in functions that operate on a NaN are not required to return a
NaN as the result". I am not entirely sure why the spec wasn't tightened
up a bit, but maybe some glsl 4+ capable hw exists which doesn't
implement d3d10 rules.

> 
>> This is enforced by d3d10 so at least d3d10 capable
>> hardware is going to honor that.
>> https://urldefense.proofpoint.com/v1/url?u=http://msdn.microsoft.com/en-us/library/windows/desktop/cc308050%28v%3Dvs.85%29.aspx&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=F4msKE2WxRzA%2BwN%2B25muztFm5TSPwE8HKJfWfR2NgfY%3D%0A&m=9dVgmTYumlj5DwQp2o0tjv4fTQpBhA%2FCDiMURzjOadM%3D%0A&s=f153cbe35b73f7fdb09fc50f8cfab56651bf346257838bc32ad35a721110fe67
>> The order of min/max though indeed matters.
> 
> Right, this is very useful information, thanks. This makes me think
> that the perhaps the dependence on the less-than definition could be
> considered a spec-bug. A quick test reveals that NVIDIA's Windows
> driver implement both min(0.5, NaN) and min(NaN, 0.5) as returning
> 0.5. So I'd say: raise an issue with Khronos, and implement DX10
> rules
> 

I agree that this looks like a bug, at the very least (given the very
lax NaN requirements in general) I think it should allow the d3d10
result if it doesn't do already (it is not obvious to me if the glsl
less-than operator really needs to adhere to ieee754 spec wrt NaNs).

Roland