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. > > commit ae2a6132b3b998b91943e4ef74fda37313f3145b > Author: Roman Lebedev <lebedev...@gmail.com> > Date: Fri Jun 15 09:56:52 2018 +0000 >
I believe the issue here is that the bfe intrinsic lowering in InstCombineCalls.cpp introduces poison values when size == 0, because it ends up generating: 'shl i32 %x, i32 32' instructions which results in a poison value, because the shift value is >= the number of bits. This commit just uncovered the problem by introducing a new sequence of instructions. -Tom > [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 <tarc...@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-sta...@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-sta...@lists.freedesktop.org >>>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-stable >>> _______________________________________________ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
