[Mesa-dev] [Mesa-stable] [PATCH] radeonsi: add a workaround for bitfield_extract when count is 0
Timothy Arceri
tarceri at itsqueeze.com
Fri Oct 26 03:04:37 UTC 2018
On 26/10/18 10:21 am, Tom Stellard wrote:
> 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?
Yes the issue is gone with that patch. Thanks!
If you let me know which LLVM versions this lands in I'll write some
patches to disable the workaround in radeonsi and radv.
>
> -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