[Mesa-dev] [PATCH 31/38] i965/fs: Migrate translation of NIR control flow to the IR builder.

[Francisco Jerez]

Matt Turner <mattst88 at gmail.com> writes:

> On Thu, Jun 4, 2015 at 9:05 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.h       |  3 ++-
>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 23 ++++++++++++-----------
>>  2 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 3f775d3..8803d5a 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -319,7 +319,8 @@ public:
>>     void nir_emit_alu(nir_alu_instr *instr);
>>     void nir_emit_intrinsic(nir_intrinsic_instr *instr);
>>     void nir_emit_texture(nir_tex_instr *instr);
>> -   void nir_emit_jump(nir_jump_instr *instr);
>> +   void nir_emit_jump(const brw::fs_builder &bld,
>> +                      nir_jump_instr *instr);
>>     fs_reg get_nir_src(nir_src src);
>>     fs_reg get_nir_dest(nir_dest dest);
>>     void emit_percomp(const brw::fs_builder &bld, const fs_inst &inst,
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index 3f84830..f31829f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -397,21 +397,21 @@ void
>>  fs_visitor::nir_emit_if(nir_if *if_stmt)
>>  {
>>     /* first, put the condition into f0 */
>> -   fs_inst *inst = emit(MOV(reg_null_d,
>> +   fs_inst *inst = bld.MOV(bld.null_reg_d(),
>>                              retype(get_nir_src(if_stmt->condition),
>> -                                   BRW_REGISTER_TYPE_D)));
>> +                                   BRW_REGISTER_TYPE_D));
>>     inst->conditional_mod = BRW_CONDITIONAL_NZ;
>>
>> -   emit(IF(BRW_PREDICATE_NORMAL));
>> +   bld.IF(BRW_PREDICATE_NORMAL);
>>
>>     nir_emit_cf_list(&if_stmt->then_list);
>>
>>     /* note: if the else is empty, dead CF elimination will remove it */
>> -   emit(BRW_OPCODE_ELSE);
>> +   bld.emit(BRW_OPCODE_ELSE);
>>
>>     nir_emit_cf_list(&if_stmt->else_list);
>>
>> -   emit(BRW_OPCODE_ENDIF);
>> +   bld.emit(BRW_OPCODE_ENDIF);
>>
>>     if (!try_replace_with_sel() && devinfo->gen < 6) {
>>        no16("Can't support (non-uniform) control flow on SIMD16\n");
>> @@ -425,11 +425,11 @@ fs_visitor::nir_emit_loop(nir_loop *loop)
>>        no16("Can't support (non-uniform) control flow on SIMD16\n");
>>     }
>>
>> -   emit(BRW_OPCODE_DO);
>> +   bld.emit(BRW_OPCODE_DO);
>>
>>     nir_emit_cf_list(&loop->body);
>>
>> -   emit(BRW_OPCODE_WHILE);
>> +   bld.emit(BRW_OPCODE_WHILE);
>>  }
>>
>>  void
>> @@ -443,6 +443,7 @@ fs_visitor::nir_emit_block(nir_block *block)
>>  void
>>  fs_visitor::nir_emit_instr(nir_instr *instr)
>>  {
>> +   const fs_builder abld = bld.annotate(NULL, instr);
>>     this->base_ir = instr;
>>
>>     switch (instr->type) {
>> @@ -465,7 +466,7 @@ fs_visitor::nir_emit_instr(nir_instr *instr)
>>        break;
>>
>>     case nir_instr_type_jump:
>> -      nir_emit_jump(nir_instr_as_jump(instr));
>> +      nir_emit_jump(abld, nir_instr_as_jump(instr));
>
> Why do we need to change the annotation to NULL?

The annotation string should already have been NULL because the
translation pass from NIR didn't ever set it to anything else -- And if
in some case it wasn't NULL, it would have been an annotation previously
set by some fs_visitor helper method during the translation of some
previous instruction "leaking" into the current instruction, IOW a bug.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150608/e107369b/attachment.sig>