Re: [R300][PATCH] Add/fix COS & SIN + FP fixes

[Rune P. <ru...@me...>]

.....

Rune Petersen wrote:
> Jerome Glisse wrote:
>> On 2/11/07, Jerome Glisse <j.g...@gm...> wrote:
>>> On 2/10/07, Rune Petersen <ru...@me...> wrote:
>>>> Hi,
>>>>
>>>> Getting proper SIN and COS wasn't as easy as it appeared. I had to make
>>>> make some changes to the fragment program code.
>>>>
>>>> general FP changes:
>>>> - support HHH swizzle for vector instructions.
>>>> - don't copy a source to a temp when it is not XYZW swizzled, but
>>>>   combine the two and have the swizzle resolve any issues.
>>>>   (saves temps/instructions with more elaborate shader code)
>>>> - Disable refcounting of temps.
>>>>   The temp R0 in progs/fp/tri-cos.c is freed prematurely.
>>>>   This should be resolved properly.
>>>> - fix overflow in cnstv[].
>>>>
>>>>
>>>> SIN & COS:
>>>> they are based on:
>>>>         http://www.devmaster.net/forums/showthread.php?t=5784
>>>>
>>>> There is an fast and a slow(high precision) version of SIN & COS.
>>>>
>>>> For SIN:
>>>> fast = 2 vector instructions
>>>> slow = 5 vector instructions
>>>>
>>>> For COS:
>>>> fast = 5 vector instructions + 2 scaler instructions
>>>> slow = 8 vector instructions + 2 scaler instructions
>>>>
>>>> The fast version appears to do a fine enough job, at least with the
>>>> simple test I have made.
>>>>
>>>>
>>>> Rune Petersen
>>> Nice to tackle this :) few comment, maybe we could make an driconf
>>> option to switch btw fast and slow version (or a more general conf
>>> option to enable or disable fragprog optimization in case we come
>>> with more optimization like that in the future).
>>>
>>> For the refcounting i am wondering if i didn't bump into that in
>>> the past, i did use gdb to trace fragprog construction at that
>>> time and found some strange interaction (which lead me to
>>> the rework i did on fragprog).
>>>
>>> Anyway here from limited testing your patch seems good,
>>> you should commit it.
>>>
>>> best,
>>> Jerome Glisse
>>>
>> Attached a patch to fix refcounting. Basicly whenever a temporary
>> source was used multiple time inside an instruction that lead to
>> multiple call to t_hw_src which is correct but as we also decrement
>> use counter in that function we over decremented the refcount.
>>
>> The patch decrement refcount after instruction decoding and avoid
>> over decrementing refcount.
>>
>> (The patch apply over yours)
>>
>> best,
>> Jerome
> 
> I have found that the main reason for my problem was I forgot to use the
> keep() on the source.
> 
> I think your patch is too intrusive. As long as keep() is used at the
> right places, you could move the refcount inside emit_arith() making the
> change more contained.
> 
> Update patch attached.
> 
> Could I get you to commit this, since I will not be able to find the
> time to figure GIT out any time (and lost my sig, don't ask).
> 
> 
> Rune Petersen
> 
>