[Mesa-dev] [Mesa-stable] [PATCH] radeonsi: add a workaround for bitfield_extract when count is 0
Tom Stellard
tstellar at redhat.com
Thu Oct 25 23:21:19 UTC 2018
On 09/27/2018 10:01 PM, Timothy Arceri wrote:
> 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.
>
Here is a proposed fix for LLVM: https://reviews.llvm.org/D53739
Would someone be able to test this?
-Tom
> 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
>>
> _______________________________________________
> 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