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

[Samuel Pitoiset]

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.

> 
>>
>> 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