[Piglit] [PATCH] gl-1.0-blend-func: skip some blend tests when using LLVM 3.8

[Roland Scheidegger]

Am 18.01.2018 um 21:04 schrieb Brian Paul:
> On 01/18/2018 12:38 PM, Roland Scheidegger wrote:
>> Am 18.01.2018 um 20:07 schrieb Brian Paul:
>>> To avoid an infinite loop.  See code comments for details.
>>> ---
>>>   tests/spec/gl-1.0/blend.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 39 insertions(+)
>>>
>>> diff --git a/tests/spec/gl-1.0/blend.c b/tests/spec/gl-1.0/blend.c
>>> index 35e940f..e69ed31 100644
>>> --- a/tests/spec/gl-1.0/blend.c
>>> +++ b/tests/spec/gl-1.0/blend.c
>>> @@ -76,6 +76,9 @@ static int test_stride = 1;
>>>   #define img_width drawing_size
>>>   #define img_height drawing_size
>>>   +static bool using_llvm_3_8 = false;
>>> +
>>> +
>>>   PIGLIT_GL_TEST_CONFIG_BEGIN
>>>         config.supports_gl_compat_version = 10;
>>> @@ -177,6 +180,10 @@ image_init(struct image *image)
>>>                GL_RGBA, GL_FLOAT, image->data);
>>>       glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
>>>       glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
>>> +
>>> +    if (strstr("LLVM 3.8", (const char *) glGetString(GL_RENDERER))
>>> == 0) {
>>> +        using_llvm_3_8 = true;
>>> +    }
>>>   } /* image_init */
>> FWIW I think this might mistakenly catch other drivers which use llvm
>> (radeonsi).
> 
> I'll fix that by also testing for "llvmpipe".
> 
> 
>>
>> I am not really sure it's worth the trouble. Personally, I couldn't care
>> less about llvm 3.8 causing a hang here (distros which updated llvm 3.8
>> with the fixes from the 3.8 stable branch won't have a problem).
> 
> As I mentioned in my other email, I recently updated to Mint 18.3
> (latest version) and this version of LLVM is the default.  Mint 18 is
> based on Ubuntu 16.04 which has LLVM 3.8 also (but I haven't tested it
> to see if has this bug or is fixed).  Same story with Ubuntu 16.10.  So
> this buggy LLVM may be common.

I thought distros would pick up fixed versions - it should be fixed in
llvm 3.8.1. If they don't, well, that's a problem. (I thought that was
pretty much the reason llvm doing stable branches nowadays, so they can
be more easily upgraded to include important fixes without having to
worry about new breakage much...).


> 
> 
>> So all
>> the trouble just for avoiding a bug with some old-ish llvm version seems
>> a bit overkill to me. I am not sure anymore if it actually was an issue
>> both on AVX(2) and non-AVX path neither.
>> Plus an actual app using the affected blend factor combinations will
>> just hang too, so hiding it here doesn't really seem helpful. (IIRC I
>> thought about trying to work around it in llvmpipe code, but there was
>> no trivial solution (for not just avoiding the hang but still correctly
>> rendering) and I didn't care enough about that particular llvm version,
>> even more so since it was fixed in the stable branch...)
>>
>> But in any case if you really think it's worthwile, I am not opposing it
>> neither, so
>> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
> 
> It'll probably save me some grief so I'd like to push it.
Go ahead then.

Roland


> 
> -Brian
> 
> 
>>
>>
>>>   @@ -532,6 +539,32 @@ apply_blend(GLenum src_factor_rgb, GLenum
>>> src_factor_a,
>>>   } /* apply_blend */
>>>     +/**
>>> + * With unpactched LLVM 3.8, llvmpipe can hit an bug in LLVM which
>>> results
>>> + * in an infinite loop.  See
>>> https://bugs.llvm.org/show_bug.cgi?id=27689
>>> + * This function tries to determine if the test case will hit that
>>> bug so
>>> + * we can skip the test.
>>> + */
>>> +bool
>>> +skip_on_llvmpipe(GLenum src_factor_rgb, GLenum src_factor_a,
>>> +                 GLenum dst_factor_rgb, GLenum dst_factor_a)
>>> +{
>>> +    if (!using_llvm_3_8)
>>> +        return false;
>>> +
>>> +    /* This could probably be a bit tighter, but this seems to catch
>>> +     * the troublesome cases.
>>> +     */
>>> +    if (src_factor_rgb == GL_CONSTANT_COLOR ||
>>> +        dst_factor_rgb == GL_CONSTANT_COLOR ||
>>> +        dst_factor_a == GL_CONSTANT_COLOR ||
>>> +        dst_factor_a == GL_CONSTANT_ALPHA)
>>> +        return true;
>>> +
>>> +    return false;
>>> +}
>>> +
>>> +
>>>   /* Test for one set of factor levels */
>>>   bool
>>>   run_factor_set(GLenum src_factor_rgb, GLenum src_factor_a,
>>> @@ -542,6 +575,12 @@ run_factor_set(GLenum src_factor_rgb, GLenum
>>> src_factor_a,
>>>       int i, j;
>>>       bool pass = true, p;
>>>   +    if (skip_on_llvmpipe(src_factor_rgb, src_factor_a,
>>> +                         dst_factor_rgb, dst_factor_a)) {
>>> +        /*printf("Skipping Blend test to avoid LLVM bug\n");*/
>>> +        return true;
>>> +    }
>>> +
>>>       glDisable(GL_DITHER);
>>>       glClear(GL_COLOR_BUFFER_BIT);
>>>  
>