[Piglit] [PATCH 2/4] arb_shader_precision: add framework for calculating tolerances for complex functions

[Ilia Mirkin]

On Tue, Feb 24, 2015 at 12:25 PM, Micah Fedke
<micah.fedke at collabora.co.uk> wrote:
>
>
> On 02/20/2015 05:31 PM, Ilia Mirkin wrote:
>>
>> On Thu, Feb 19, 2015 at 1:06 AM, Micah Fedke
>> <micah.fedke at collabora.co.uk> wrote:
>>>
>>> +def _gen_tolerance(name, rettype, args):
>>> +    """Return the tolerance that should be allowed for a function for
>>> the
>>> +    test vector passed in.  Return -1 for any vectors that would push
>>> the
>>> +    tolerance outside of acceptable bounds
>>> +    """
>>> +    if name in simple_fns:
>>> +        if name == 'op-mult' or name == 'op-assign-mult':
>>
>>
>> Seems like this should be outside of the if. So like
>>
>> if name == 'op-mult' or name == ...:
>> elif name in simple_fns:
>> elif name in compelx_fns:
>>
>
> It could work just as well that way, yes.  My intent was to first split the
> names into two mutually exclusive high-level groups (simple_fns and
> complex_fns), and then treat individual members of those groups as necessary
> (eg. the mult operations, which only appear within simple_fns).  I'd like to
> leave it this way for clarity, if you agree? I can add a comment line
> somewhere about the mutual exclusivity of the two groups.

Yeah, but you treat op-mult totally differently than the other simple
functions. Making them not-simple. Perhaps they should just be in
complex fns?

>
>>> +            x_type = glsl_type_of(args[0])
>>> +            y_type = glsl_type_of(args[1])
>>> +            if x_type.is_vector and y_type.is_matrix:
>>> +                mult_func = _vec_times_mat_ref
>>> +            elif x_type.is_matrix and y_type.is_vector:
>>> +                mult_func = _mat_times_vec_ref
>>> +            elif x_type.is_matrix and y_type.is_matrix:
>>> +                mult_func = _mat_times_mat_ref
>>> +            else:
>>> +                return simple_fns[name]
>>> +            ret = _analyze_ref_fn(mult_func, args)
>>> +            return -1.0 if any(ret['badlands']) else map(float,
>>> ret['component_tolerances'])
>>> +        else:
>>> +            return simple_fns[name]
>>> +    elif name in complex_fns:
>>> +        if name in componentwise_fns:
>>> +            ret = {'errors':[], 'badlands':[],
>>> 'component_tolerances':[]}
>>
>>
>> Is there some sort of advantage to keeping these in a dict?
>
>
> The dict is for clarity purposes only.  I guess I could use a named tuple
> instead?  It seemed like more code . . .

The alternative I had in mind was just 3 separate variables...