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

[Vadim Girlin]

On 08/24/2013 07:24 PM, Marek Olšák wrote:
> See piglit/fbo-alphatest-nocolor.

Ah, it seems I just compared wrong results when I was testing all 
combinations of backends and looked for regressions.

Now I think the problem is that even though llvm backend correctly emits 
color export with nr_cbufs==0, but it still relies on 
nr_ps_color_exports value computed in the old backend path (which is 
currently broken for that case), and this resulted in the regressions 
that I wanted to fix. I'll send new patch.

Vadim

>
> Marek
>
> On Sat, Aug 24, 2013 at 3:12 PM, Vadim Girlin <vadimgirlin at gmail.com> wrote:
>> On 08/24/2013 02:31 PM, Marek Olšák wrote:
>>>
>>> Like Christoph said, COLOR0 (if available) must always be exported for
>>> alpha test.
>>
>>
>> Are there any piglit tests for that? I didn't see any regressions with this
>> patch (at least on evergreen), possibly I messed up the testing somehow.
>> Also I think old backend uses the same logic.
>>
>> Vadim
>>
>>
>>>
>>> Marek
>>>
>>> On Sat, Aug 24, 2013 at 3:30 AM, Vadim Girlin <vadimgirlin at gmail.com>
>>> wrote:
>>>>
>>>> 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>
>>>> ---
>>>>
>>>> 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;
>>>> --
>>>> 1.8.3.1
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>