[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 <sroland at vmware.com>


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