[Mesa-dev] [PATCH 3/3] ac: make use of if/loop build helpers

Alex Smith asmith at feralinteractive.com
Tue Apr 3 09:58:15 UTC 2018


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



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.freedes
>>> ktop.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
>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180403/6aa0adf7/attachment-0001.html>


More information about the mesa-dev mailing list