[Mesa-dev] [PATCH] r600g/llvm: don't export more colors than the number of CBs

[Christian König]

Am 24.08.2013 03:30, schrieb Vadim Girlin:
> Currently llvm backend always exports at least one color in pixel
> shader even if no color buffers are enabled. With depth/stencil exports
> this can result in the following code:
>
> EXPORT             PIXEL 0     R0.xyzw  VPM
> EXPORT             PIXEL 61    R1.x___  VPM
> EXPORT_DONE        PIXEL 61    R0._x__  VPM  EOP
>
> AFAIU with zero color buffers no memory is reserved for colors in the export
> ring and all exports in this example actually write to the same location.
> The code above still works fine in this particular case, because correct
> values are written last, but reordering can break it (especially with SB
> which tends to reorder the exports).
>
> Signed-off-by: Vadim Girlin <vadimgirlin at gmail.com>

I briefly remember that we needed at least one color export otherwise 
the GPU might hang, but I'm not 100% sure of that.

Marek and Alex should probably also take a look on this before we commit it.

Christian.

> ---
>
> This fixes regressions with LLVM+SB, so I consider it as a prerequisite
> for enabling SB by default. Also it fixes some issues with LLVM backend alone.
> Tested on evergreen only (I don't have other hw), needs testing on
> pre-evergreen GPUs.
>
>   src/gallium/drivers/r600/r600_llvm.c   | 2 +-
>   src/gallium/drivers/r600/r600_shader.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_llvm.c b/src/gallium/drivers/r600/r600_llvm.c
> index 03a68e4..d2f4aff 100644
> --- a/src/gallium/drivers/r600/r600_llvm.c
> +++ b/src/gallium/drivers/r600/r600_llvm.c
> @@ -333,8 +333,8 @@ static void llvm_emit_epilogue(struct lp_build_tgsi_context * bld_base)
>   		} else if (ctx->type == TGSI_PROCESSOR_FRAGMENT) {
>   			switch (ctx->r600_outputs[i].name) {
>   			case TGSI_SEMANTIC_COLOR:
> -				has_color = true;
>   				if ( color_count < ctx->color_buffer_count) {
> +					has_color = true;
>   					LLVMValueRef args[3];
>   					args[0] = output;
>   					if (ctx->fs_color_all) {
> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
> index fb766c4..85f8469 100644
> --- a/src/gallium/drivers/r600/r600_shader.c
> +++ b/src/gallium/drivers/r600/r600_shader.c
> @@ -1130,7 +1130,7 @@ static int r600_shader_from_tgsi(struct r600_screen *rscreen,
>   		radeon_llvm_ctx.face_gpr = ctx.face_gpr;
>   		radeon_llvm_ctx.r600_inputs = ctx.shader->input;
>   		radeon_llvm_ctx.r600_outputs = ctx.shader->output;
> -		radeon_llvm_ctx.color_buffer_count = MAX2(key.nr_cbufs , 1);
> +		radeon_llvm_ctx.color_buffer_count = key.nr_cbufs;
>   		radeon_llvm_ctx.chip_class = ctx.bc->chip_class;
>   		radeon_llvm_ctx.fs_color_all = shader->fs_write_all && (rscreen->chip_class >= EVERGREEN);
>   		radeon_llvm_ctx.stream_outputs = &so;