[Mesa-dev] [Mesa-stable] [PATCH] radeonsi: add a workaround for bitfield_extract when count is 0

Timothy Arceri tarceri at itsqueeze.com
Fri Sep 28 05:01:11 UTC 2018


On 28/9/18 2:53 am, Tom Stellard wrote:
> On 09/24/2018 11:51 PM, Samuel Pitoiset wrote:
>>
>>
>> On 9/25/18 6:46 AM, Timothy Arceri wrote:
>>> On 25/9/18 10:35 am, Marek Olšák wrote:
>>>> Do you know what's broken in LLVM? Or is it a problem with the ISA?
>>>
>>> I haven't actually dug any further. Adding Samuel to see if he remembers more.
>>>
>>> However according to the original bug report this is a regression when going from LLVM 6 to LLVM 7. I see the issue on both polaris and vega.
>>
>> I didn't investigate either, it was just simpler to add a workaround in mesa than fixing LLVM.
> 
> If you have a test case, I can take a look.  There is still a lot of time
> before 7.0.1 is released.

Hi Tom,

There is a renderdoc capture [1] attached to a RADV bug and an apitrace 
[2] attached to a seemingly unrelated radeonsi (VEGA) bug.

In the apitrace from Civ VI there is incorrect rendering behind the 
chinese emperor on the loading screen.

I've bisected the problem, reverting the following llvm commit fixes the 
issue.

commit ae2a6132b3b998b91943e4ef74fda37313f3145b
Author: Roman Lebedev <lebedev.ri at gmail.com>
Date:   Fri Jun 15 09:56:52 2018 +0000

     [InstCombine] Recommit: Fold  (x << y) >> y  ->  x & (-1 >> y)

     Summary:
     We already do it for splat constants, but not just values.
     Also, undef cases are mostly non-functional.

     The original commit was reverted because
     it broke tests for amdgpu backend, which i didn't check.
     Now, the backed was updated to recognize these new
     patterns, so we are good.

     https://bugs.llvm.org/show_bug.cgi?id=37603
     https://rise4fun.com/Alive/cplX

     Reviewers: spatel, craig.topper, mareko, bogner, rampitec, 
nhaehnle, arsenm

     Reviewed By: spatel, rampitec, nhaehnle

     Subscribers: wdng, nhaehnle, llvm-commits

     Differential Revision: https://reviews.llvm.org/D47980

     git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@334818 
91177308-0d34-0410-b5e6-96231b3b80d8

[1] https://bugs.freedesktop.org/show_bug.cgi?id=107276
[2] https://bugs.freedesktop.org/show_bug.cgi?id=104602

> 
> -Tom
> 
>>
>>>
>>>>
>>>> Marek
>>>>
>>>> On Fri, Sep 21, 2018 at 10:38 PM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>>>>> This ports the fix from 3d41757788ac. Both LLVM 7 & 8 continue
>>>>> to have this problem.
>>>>>
>>>>> It fixes rendering issues in some menu and loading screens of
>>>>> Civ VI which can be seen in the trace from bug 104602.
>>>>>
>>>>> Note: This does not fix the black triangles on Vega for bug
>>>>> 104602.
>>>>>
>>>>> Cc: mesa-stable at lists.freedesktop.org
>>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104602
>>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107276
>>>>> ---
>>>>>    .../drivers/radeonsi/si_shader_tgsi_alu.c     | 41 ++++++++++++++-----
>>>>>    1 file changed, 30 insertions(+), 11 deletions(-)
>>>>>
>>>>> diff --git a/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c b/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c
>>>>> index f54d025aec0..814362bc963 100644
>>>>> --- a/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c
>>>>> +++ b/src/gallium/drivers/radeonsi/si_shader_tgsi_alu.c
>>>>> @@ -494,18 +494,37 @@ static void emit_bfe(const struct lp_build_tgsi_action *action,
>>>>>                        struct lp_build_emit_data *emit_data)
>>>>>    {
>>>>>           struct si_shader_context *ctx = si_shader_context(bld_base);
>>>>> -       LLVMValueRef bfe_sm5;
>>>>> -       LLVMValueRef cond;
>>>>> -
>>>>> -       bfe_sm5 = ac_build_bfe(&ctx->ac, emit_data->args[0],
>>>>> -                              emit_data->args[1], emit_data->args[2],
>>>>> -                              emit_data->info->opcode == TGSI_OPCODE_IBFE);
>>>>>
>>>>> -       /* Correct for GLSL semantics. */
>>>>> -       cond = LLVMBuildICmp(ctx->ac.builder, LLVMIntUGE, emit_data->args[2],
>>>>> -                            LLVMConstInt(ctx->i32, 32, 0), "");
>>>>> -       emit_data->output[emit_data->chan] =
>>>>> -               LLVMBuildSelect(ctx->ac.builder, cond, emit_data->args[0], bfe_sm5, "");
>>>>> +       if (HAVE_LLVM < 0x0700) {
>>>>> +               LLVMValueRef bfe_sm5 =
>>>>> +                       ac_build_bfe(&ctx->ac, emit_data->args[0],
>>>>> +                                    emit_data->args[1], emit_data->args[2],
>>>>> +                                    emit_data->info->opcode == TGSI_OPCODE_IBFE);
>>>>> +
>>>>> +               /* Correct for GLSL semantics. */
>>>>> +               LLVMValueRef cond = LLVMBuildICmp(ctx->ac.builder, LLVMIntUGE, emit_data->args[2],
>>>>> +                                                 LLVMConstInt(ctx->i32, 32, 0), "");
>>>>> +               emit_data->output[emit_data->chan] =
>>>>> +                       LLVMBuildSelect(ctx->ac.builder, cond, emit_data->args[0], bfe_sm5, "");
>>>>> +       } else {
>>>>> +               /* FIXME: LLVM 7 returns incorrect result when count is 0.
>>>>> +                * https://bugs.freedesktop.org/show_bug.cgi?id=107276
>>>>> +                */
>>>>> +               LLVMValueRef zero = ctx->i32_0;
>>>>> +               LLVMValueRef bfe_sm5 =
>>>>> +                       ac_build_bfe(&ctx->ac, emit_data->args[0],
>>>>> +                                    emit_data->args[1], emit_data->args[2],
>>>>> +                                    emit_data->info->opcode == TGSI_OPCODE_IBFE);
>>>>> +
>>>>> +               /* Correct for GLSL semantics. */
>>>>> +               LLVMValueRef cond = LLVMBuildICmp(ctx->ac.builder, LLVMIntUGE, emit_data->args[2],
>>>>> +                                                 LLVMConstInt(ctx->i32, 32, 0), "");
>>>>> +               LLVMValueRef cond2 = LLVMBuildICmp(ctx->ac.builder, LLVMIntEQ, emit_data->args[2],
>>>>> +                                                  zero, "");
>>>>> +               bfe_sm5 = LLVMBuildSelect(ctx->ac.builder, cond, emit_data->args[0], bfe_sm5, "");
>>>>> +               emit_data->output[emit_data->chan] =
>>>>> +                       LLVMBuildSelect(ctx->ac.builder, cond2, zero, bfe_sm5, "");
>>>>> +       }
>>>>>    }
>>>>>
>>>>>    /* this is ffs in C */
>>>>> -- 
>>>>> 2.17.1
>>>>>
>>>>> _______________________________________________
>>>>> mesa-stable mailing list
>>>>> mesa-stable at lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 


More information about the mesa-dev mailing list