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

[Roland Scheidegger]

Am 19.01.2018 um 06:35 schrieb Eric Anholt:
> Brian Paul  writes:
> 
>> On 01/18/2018 01:27 PM, Eric Anholt wrote:
>>> Brian Paul  writes:
>>>
 To avoid an infinite loop.  See code comments for details.
>>>
>>> Skipping a failing test and returning pass is wrong to me.
>>
>> It's not ideal.  But the bug is in LLVM and cannot readily be fixed in 
>> llvmpipe.
>>
>> I could have the test return a WARN result in this situation.  Would 
>> that be better?
> 
> It's still a bug in the driver, even if it's because the driver's using
> a buggy external library.  It should be a fail.
> 

Albeit it's just a guess it will hang. With a fixed llvm 3.8 from the
stable branch, it would not hang and pass. Or IIRC if you don't have a
avx-capable cpu, it also would not hang (and with a non-x86 cpu it won't
hang neither).
But I don't really care either way if it just reports fail in this case.
(Would be nice if we could just determine the hang empirically I
suppose, if the testcase runs for more than a second kill it and it's a
fail, but that doesn't work easily.)

Roland
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

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

[Eric Anholt]

Brian Paul  writes:

> On 01/18/2018 01:27 PM, Eric Anholt wrote:
>> Brian Paul  writes:
>> 
>>> To avoid an infinite loop.  See code comments for details.
>> 
>> Skipping a failing test and returning pass is wrong to me.
>
> It's not ideal.  But the bug is in LLVM and cannot readily be fixed in 
> llvmpipe.
>
> I could have the test return a WARN result in this situation.  Would 
> that be better?

It's still a bug in the driver, even if it's because the driver's using
a buggy external library.  It should be a fail.


signature.asc
Description: PGP signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

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

[Brian Paul]

On 01/18/2018 01:27 PM, Eric Anholt wrote:

Brian Paul  writes:


To avoid an infinite loop.  See code comments for details.


Skipping a failing test and returning pass is wrong to me.


It's not ideal.  But the bug is in LLVM and cannot readily be fixed in 
llvmpipe.


I could have the test return a WARN result in this situation.  Would 
that be better?


-Brian
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [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 
> 
> 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);
>>>  
> 

___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

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

[Eric Anholt]

Brian Paul  writes:

> To avoid an infinite loop.  See code comments for details.

Skipping a failing test and returning pass is wrong to me.


signature.asc
Description: PGP signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

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

[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.




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 


It'll probably save me some grief so I'd like to push it.

-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);
  



___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

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

[Roland Scheidegger]

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 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). 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 


>  
> @@ -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);
>  
> 
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

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

[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 */
 
 
@@ -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);
 
-- 
2.7.4

___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit