[Mesa-dev] [PATCH 3/3] ac: make use of if/loop build helpers
Timothy Arceri
tarceri at itsqueeze.com
Tue Apr 3 09:45:38 UTC 2018
I have no issue with these going in stable if they fix bugs. Ideally we
should create a piglit test to catch this also but presumably you guys
don't actually know the exact shader combination thats tripping things up?
On 03/04/18 19:36, Samuel Pitoiset wrote:
> This fixes a rendering issue with Wolfenstein 2 as well. A backport
> sounds reasonable to me.
>
> On 04/03/2018 11:33 AM, Alex Smith wrote:
>> Hi Timothy,
>>
>> This patch fixes some rendering issues I see with RADV on SI.
>>
>> It doesn't sound like it was really intended to fix anything, so
>> possibly it's masking some other issue, but would you object to
>> nominating the series for stable? Applying it on the 18.0 branch fixes
>> the issue there as well.
>>
>> Thanks,
>> Alex
>>
>> On 7 March 2018 at 20:43, Marek Olšák <maraeo at gmail.com
>> <mailto:maraeo at gmail.com>> wrote:
>>
>> For the series:
>>
>> Reviewed-by: Marek Olšák <marek.olsak at amd.com
>> <mailto:marek.olsak at amd.com>>
>>
>> Marek
>>
>> On Tue, Mar 6, 2018 at 8:40 PM, Timothy Arceri
>> <tarceri at itsqueeze.com <mailto:tarceri at itsqueeze.com>> wrote:
>> > These helpers insert the basic block in the same order as they
>> > appear in NIR making it easier to follow LLVM IR dumps. The
>> helpers
>> > also insert more useful labels onto the blocks.
>> >
>> > TGSI use the line number of the corresponding opcode in the TGSI
>> > dump as the label id, here we use the corresponding block index
>> > from NIR.
>> > ---
>> > src/amd/common/ac_nir_to_llvm.c | 60
>> +++++++++++++----------------------------
>> > 1 file changed, 18 insertions(+), 42 deletions(-)
>> >
>> > diff --git a/src/amd/common/ac_nir_to_llvm.c
>> b/src/amd/common/ac_nir_to_llvm.c
>> > index cda91fe8bf..dc463ed253 100644
>> > --- a/src/amd/common/ac_nir_to_llvm.c
>> > +++ b/src/amd/common/ac_nir_to_llvm.c
>> > @@ -5237,17 +5237,15 @@ static void visit_ssa_undef(struct
>> ac_nir_context *ctx,
>> > _mesa_hash_table_insert(ctx->defs, &instr->def, undef);
>> > }
>> >
>> > -static void visit_jump(struct ac_nir_context *ctx,
>> > +static void visit_jump(struct ac_llvm_context *ctx,
>> > const nir_jump_instr *instr)
>> > {
>> > switch (instr->type) {
>> > case nir_jump_break:
>> > - LLVMBuildBr(ctx->ac.builder, ctx->break_block);
>> > - LLVMClearInsertionPosition(ctx->ac.builder);
>> > + ac_build_break(ctx);
>> > break;
>> > case nir_jump_continue:
>> > - LLVMBuildBr(ctx->ac.builder, ctx->continue_block);
>> > - LLVMClearInsertionPosition(ctx->ac.builder);
>> > + ac_build_continue(ctx);
>> > break;
>> > default:
>> > fprintf(stderr, "Unknown NIR jump instr: ");
>> > @@ -5285,7 +5283,7 @@ static void visit_block(struct
>> ac_nir_context *ctx, nir_block *block)
>> > visit_ssa_undef(ctx,
>> nir_instr_as_ssa_undef(instr));
>> > break;
>> > case nir_instr_type_jump:
>> > - visit_jump(ctx, nir_instr_as_jump(instr));
>> > + visit_jump(&ctx->ac,
>> nir_instr_as_jump(instr));
>> > break;
>> > default:
>> > fprintf(stderr, "Unknown NIR instr
>> type: ");
>> > @@ -5302,56 +5300,34 @@ static void visit_if(struct
>> ac_nir_context *ctx, nir_if *if_stmt)
>> > {
>> > LLVMValueRef value = get_src(ctx, if_stmt->condition);
>> >
>> > - LLVMValueRef fn =
>> LLVMGetBasicBlockParent(LLVMGetInsertBlock(ctx->ac.builder));
>> > - LLVMBasicBlockRef merge_block =
>> > - LLVMAppendBasicBlockInContext(ctx->ac.context, fn,
>> "");
>> > - LLVMBasicBlockRef if_block =
>> > - LLVMAppendBasicBlockInContext(ctx->ac.context, fn,
>> "");
>> > - LLVMBasicBlockRef else_block = merge_block;
>> > - if (!exec_list_is_empty(&if_stmt->else_list))
>> > - else_block = LLVMAppendBasicBlockInContext(
>> > - ctx->ac.context, fn, "");
>> > -
>> > - LLVMValueRef cond = LLVMBuildICmp(ctx->ac.builder,
>> LLVMIntNE, value,
>> > - ctx->ac.i32_0, "");
>> > - LLVMBuildCondBr(ctx->ac.builder, cond, if_block,
>> else_block);
>> > -
>> > - LLVMPositionBuilderAtEnd(ctx->ac.builder, if_block);
>> > + nir_block *then_block =
>> > + (nir_block *)
>> exec_list_get_head(&if_stmt->then_list);
>> > +
>> > + ac_build_uif(&ctx->ac, value, then_block->index);
>> > +
>> > visit_cf_list(ctx, &if_stmt->then_list);
>> > - if (LLVMGetInsertBlock(ctx->ac.builder))
>> > - LLVMBuildBr(ctx->ac.builder, merge_block);
>> >
>> > if (!exec_list_is_empty(&if_stmt->else_list)) {
>> > - LLVMPositionBuilderAtEnd(ctx->ac.builder,
>> else_block);
>> > + nir_block *else_block =
>> > + (nir_block *)
>> exec_list_get_head(&if_stmt->else_list);
>> > +
>> > + ac_build_else(&ctx->ac, else_block->index);
>> > visit_cf_list(ctx, &if_stmt->else_list);
>> > - if (LLVMGetInsertBlock(ctx->ac.builder))
>> > - LLVMBuildBr(ctx->ac.builder, merge_block);
>> > }
>> >
>> > - LLVMPositionBuilderAtEnd(ctx->ac.builder, merge_block);
>> > + ac_build_endif(&ctx->ac, then_block->index);
>> > }
>> >
>> > static void visit_loop(struct ac_nir_context *ctx, nir_loop
>> *loop)
>> > {
>> > - LLVMValueRef fn =
>> LLVMGetBasicBlockParent(LLVMGetInsertBlock(ctx->ac.builder));
>> > - LLVMBasicBlockRef continue_parent = ctx->continue_block;
>> > - LLVMBasicBlockRef break_parent = ctx->break_block;
>> > + nir_block *first_loop_block =
>> > + (nir_block *) exec_list_get_head(&loop->body);
>> >
>> > - ctx->continue_block =
>> > - LLVMAppendBasicBlockInContext(ctx->ac.context, fn,
>> "");
>> > - ctx->break_block =
>> > - LLVMAppendBasicBlockInContext(ctx->ac.context, fn,
>> "");
>> > + ac_build_bgnloop(&ctx->ac, first_loop_block->index);
>> >
>> > - LLVMBuildBr(ctx->ac.builder, ctx->continue_block);
>> > - LLVMPositionBuilderAtEnd(ctx->ac.builder,
>> ctx->continue_block);
>> > visit_cf_list(ctx, &loop->body);
>> >
>> > - if (LLVMGetInsertBlock(ctx->ac.builder))
>> > - LLVMBuildBr(ctx->ac.builder, ctx->continue_block);
>> > - LLVMPositionBuilderAtEnd(ctx->ac.builder,
>> ctx->break_block);
>> > -
>> > - ctx->continue_block = continue_parent;
>> > - ctx->break_block = break_parent;
>> > + ac_build_endloop(&ctx->ac, first_loop_block->index);
>> > }
>> >
>> > static void visit_cf_list(struct ac_nir_context *ctx,
>> > --
>> > 2.14.3
>> >
>> > _______________________________________________
>> > mesa-dev mailing list
>> > mesa-dev at lists.freedesktop.org
>> <mailto:mesa-dev at lists.freedesktop.org>
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> <mailto:mesa-dev at lists.freedesktop.org>
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> <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