[Mesa-dev] [PATCH] r600g: fix color exports when we have no CBs

[Vadim Girlin]

On 08/27/2013 11:00 PM, Roland Scheidegger wrote:
> Not that I'm qualified to review r600 code, but couldn't you create
> different shader variants depending on whether you need alpha test? At
> least I would assume shader exports aren't free.

I thought about performance, but my main concern now is to avoid serious 
regressions after enabling sb, we can try to improve it later.

Even if we won't emit this color export, we'll have fake export (with 
all color components masked) instead, and I'm not sure whether it's 
cheaper. Possibly hardware can see that there is no actual memory write, 
but benchmarks are needed to prove it.

Also there is another possible improvement for exports - sometimes we 
need to export depth/stencil but no colors, probably we can get rid of 
fake color export as well in such cases. Anyway, this also needs 
additional testing/benchmarking.

Vadim

>
> Roland
>
> Am 27.08.2013 19:56, schrieb Vadim Girlin:
>> We need to export at least one color if the shader writes it,
>> even when nr_cbufs==0.
>>
>> Signed-off-by: Vadim Girlin <vadimgirlin at gmail.com>
>> ---
>>
>> Tested on evergreen with multiple combinations of backends - no regressions,
>> fixes some tests:
>>
>>    default    - fixes fb-alphatest-nocolor and fb_alphatest-nocolor-ff
>>    default+sb - fixes fb-alphatest-nocolor and fb_alphatest-nocolor-ff
>>    llvm       - fixes about 25 tests related to depth/stencil
>>    llvm+sb    - fixes about 300 tests (llvm's depth/stencil issues and
>>                 regressions cased by reordering of exports in sb)
>>
>> With this patch, there are no regressions with default+sb vs default.
>> There is one regression with llvm+sb vs llvm - fs-texturegrad-miplevels,
>> AFAICS it's a problem with llvm backend uncovered by sb - SET_GRADIENTS_V/H
>> instructions are not placed in the same TEX clause with corresponding SAMPLE_G.
>>
>>   src/gallium/drivers/r600/r600_shader.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
>> index 300b5c4..f7eab76 100644
>> --- a/src/gallium/drivers/r600/r600_shader.c
>> +++ b/src/gallium/drivers/r600/r600_shader.c
>> @@ -918,6 +918,7 @@ static int r600_shader_from_tgsi(struct r600_screen *rscreen,
>>   	unsigned opcode;
>>   	int i, j, k, r = 0;
>>   	int next_pos_base = 60, next_param_base = 0;
>> +	int max_color_exports = MAX2(key.nr_cbufs, 1);
>>   	/* Declarations used by llvm code */
>>   	bool use_llvm = false;
>>   	bool indirect_gprs;
>> @@ -1130,7 +1131,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 = max_color_exports;
>>   		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;
>> @@ -1440,7 +1441,7 @@ static int r600_shader_from_tgsi(struct r600_screen *rscreen,
>>   		case TGSI_PROCESSOR_FRAGMENT:
>>   			if (shader->output[i].name == TGSI_SEMANTIC_COLOR) {
>>   				/* never export more colors than the number of CBs */
>> -				if (shader->output[i].sid >= key.nr_cbufs) {
>> +				if (shader->output[i].sid >= max_color_exports) {
>>   					/* skip export */
>>   					j--;
>>   					continue;
>> @@ -1450,7 +1451,7 @@ static int r600_shader_from_tgsi(struct r600_screen *rscreen,
>>   				output[j].type = V_SQ_CF_ALLOC_EXPORT_WORD0_SQ_EXPORT_PIXEL;
>>   				shader->nr_ps_color_exports++;
>>   				if (shader->fs_write_all && (rscreen->chip_class >= EVERGREEN)) {
>> -					for (k = 1; k < key.nr_cbufs; k++) {
>> +					for (k = 1; k < max_color_exports; k++) {
>>   						j++;
>>   						memset(&output[j], 0, sizeof(struct r600_bytecode_output));
>>   						output[j].gpr = shader->output[i].gpr;
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>