[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