[Mesa-dev] [Mesa-stable] [PATCH 3/3] ac: make use of if/loop build helpers
Juan A. Suarez Romero
jasuarez at igalia.com
Wed Apr 11 14:03:42 UTC 2018
On Tue, 2018-04-10 at 16:48 +0100, Alex Smith wrote:
> On 10 April 2018 at 15:49, Juan A. Suarez Romero <jasuarez at igalia.com> wrote:
> > On Tue, 2018-04-03 at 10:58 +0100, Alex Smith wrote:
> > > I don't know exactly what's causing it, no. I noticed the issue was fixed on master so just bisected to this.
> > >
> > > CC'ing stable to nominate:
> > > 42627dabb4db3011825a022325be7ae9b51103d6 - (1/3) ac: add if/loop build helpers
> > > 6e1a142863b368a032e333f09feb107241446053 - (2/3) radeonsi: make use of if/loop build helpers in ac
> > > 99cdc019bf6fe11c135b7544ef6daf4ac964fa24 - (3/3) ac: make use of if/loop build helpers
> > >
> >
> > Hi, Alex.
> >
> > Are these 3 commits nominated for a specific stable branch? From the CC not sure
> > if you want to nominate them for 17.3, 18.0 or both.
>
> They work for me on both 18.0 and 17.3, so I think they can be nominated for both.
>
Thanks. Enqueued them for next 17.3 (last) and 18.0 stable releases.
J.A.
> Thanks,
> Alex
>
> >
> > J.A.
> >
> > >
> > >
> > > On 3 April 2018 at 10:45, Timothy Arceri <tarceri at itsqueeze.com> wrote:
> > > > 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
> > > > > >
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
More information about the mesa-dev
mailing list