[Mesa-dev] [PATCH] draw/llvm: fix generation of the VS with GS present

[Matthew McClure]

Looks good to me too!

Reviewed-by: Matthew McClure <mcclurem at vmware.com>

Matthew

----- Original Message -----
From: "Roland Scheidegger" <sroland at vmware.com>
To: "Zack Rusin" <zackr at vmware.com>, mesa-dev at lists.freedesktop.org
Cc: jfonseca at vmware.com, mcclurem at vmware.com
Sent: Tuesday, March 4, 2014 8:57:30 AM
Subject: Re: [PATCH] draw/llvm: fix generation of the VS with GS present

Am 04.03.2014 05:08, schrieb Zack Rusin:
> draw_current_shader_* functions return a final output when considering
> both the geometry shader and the vertex shader. But when code generating
> vertex shader we can not be using output slots from the geometry shader
> because, obviously, those can be completely different. This fixes a
> number of very non-obvious crashes.
> A side-effect of this bug was that sometimes the vertex shading code
> could save some random outputs as position/clip when the geometry
> shader was writing them and vertex shader had different outputs at
> those slots (sometimes writing garbage and sometimes something correct).
> ---
>  src/gallium/auxiliary/draw/draw_llvm.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c
> index 0bbb680..53d13f3 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> @@ -1104,7 +1104,7 @@ generate_viewport(struct draw_llvm_variant *variant,
>     int i;
>     struct gallivm_state *gallivm = variant->gallivm;
>     struct lp_type f32_type = vs_type;
> -   const unsigned pos = draw_current_shader_position_output(variant->llvm->draw);
> +   const unsigned pos = variant->llvm->draw->vs.position_output;
>     LLVMTypeRef vs_type_llvm = lp_build_vec_type(gallivm, vs_type);
>     LLVMValueRef out3 = LLVMBuildLoad(builder, outputs[pos][3], ""); /*w0 w1 .. wn*/
>     LLVMValueRef const1 = lp_build_const_vec(gallivm, f32_type, 1.0);       /*1.0 1.0 1.0 1.0*/
> @@ -1173,14 +1173,14 @@ generate_clipmask(struct draw_llvm *llvm,
>     LLVMValueRef plane1, planes, plane_ptr, sum;
>     struct lp_type f32_type = vs_type;
>     struct lp_type i32_type = lp_int_type(vs_type);
> -   const unsigned pos = draw_current_shader_position_output(llvm->draw);
> -   const unsigned cv = draw_current_shader_clipvertex_output(llvm->draw);
> +   const unsigned pos = llvm->draw->vs.position_output;
> +   const unsigned cv = llvm->draw->vs.clipvertex_output;
>     int num_written_clipdistance = llvm->draw->vs.vertex_shader->info.num_written_clipdistance;
>     bool have_cd = false;
>     unsigned cd[2];
>  
> -   cd[0] = draw_current_shader_clipdistance_output(llvm->draw, 0);
> -   cd[1] = draw_current_shader_clipdistance_output(llvm->draw, 1);
> +   cd[0] = llvm->draw->vs.clipdistance_output[0];
> +   cd[1] = llvm->draw->vs.clipdistance_output[1];
>  
>     if (cd[0] != pos || cd[1] != pos)
>        have_cd = true;
> @@ -1551,8 +1551,8 @@ draw_llvm_generate(struct draw_llvm *llvm, struct draw_llvm_variant *variant,
>                                             key->clip_z  ||
>                                             key->clip_user);
>     LLVMValueRef variant_func;
> -   const unsigned pos = draw_current_shader_position_output(llvm->draw);
> -   const unsigned cv = draw_current_shader_clipvertex_output(llvm->draw);
> +   const unsigned pos = llvm->draw->vs.position_output;
> +   const unsigned cv = llvm->draw->vs.clipvertex_output;
>     boolean have_clipdist = FALSE;
>     struct lp_bld_tgsi_system_values system_values;
>  
> 

Both patches make sense to me, good catch!

Roland