Re: [Piglit] [PATCH] gl-1.0-blend-func: skip some blend tests when using LLVM 3.8
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
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
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
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
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
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
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
