[Mesa-stable] [Mesa-dev] [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-stable
mailing list