[Mesa-dev] shader_test for glsl-to-tgsi ceil bug

[Christoph Bumiller]

On 05/08/2012 05:33 PM, Ian Romanick wrote:
> On 05/07/2012 04:16 PM, Christoph Bumiller wrote:
>> On 05/07/2012 08:34 PM, Eric Anholt wrote:
>>> On Sat, 05 May 2012 14:43:44 +0200, Christoph Bumiller
>>> <e0425955 at student.tuwien.ac.at>  wrote:
>>>> Test case for the "glsl_to_tgsi: use TGSI_OPCODE_CEIL for
>>>> ir_unop_ceil" patch attached.
>>>
>>> This wasn't caught by the generated test for ceil()?  That seems
>>> strange.
>> It's not, because it's not really meant to test the functionality of
>> ceil() itself, but rather a bug in glsl-to-tgsi, which implemented it
>> as -floor(-x), that causes the second negation to be lost if the
>> result was used in a certain way, like the reciprocal here.
>>
>> I just sent the test along to illustrate the bug my patch was supposed
>> to fix, although including those kinds of failures in piglit would
>> probably be a good idea, too.
>>
>> Of course testing multiple values, including fractional ones, won't
>> harm either, even if it doesn't make a difference for that specific bug.
> 
> We *have* a bunch of ceil tests in piglit.  What Eric wanted to know was
> whether or not the existing tests already reproduce the failure.  If
> they do not, then they should be extended.
> 
> 

I know, and I said (meant to, anyway, sorry if I wasn't clear) that it's
not *about* ceil, and of course they all pass, or I wouldn't have made
an extra test but just referenced the existing one instead.

As for adding this test to piglit, nevermind. On second thought it would
make more sense to investigate the structure of the glsl-to-tgsi bug and
tailor a test to that in the hopes of covering more potential cases.


-- less essential part --
Anyway, I don't really know how the glsl-to-tgsi code works and I don't
have the time (more the nerve) to learn it right now, I just wanted it
to work.
Granted, if I did, I could check the rest of the code for equivalent
mistakes, but, it would be more efficient if someone familiar with the
code did that.
So, anyone else working on it, I hope you see this and don't plant the
same bug elsewhere.