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

[Tom Stellard]

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.

-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