[Mesa-dev] [PATCH 34/38] i965/fs: Migrate translation of NIR texturing instructions 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 | 11 ++++-------
>>  2 files changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 338c816..ef0256d 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(const brw::fs_builder &bld, nir_alu_instr *instr);
>>     void nir_emit_intrinsic(const brw::fs_builder &bld,
>>                             nir_intrinsic_instr *instr);
>> -   void nir_emit_texture(nir_tex_instr *instr);
>> +   void nir_emit_texture(const brw::fs_builder &bld,
>> +                         nir_tex_instr *instr);
>>     void nir_emit_jump(const brw::fs_builder &bld,
>>                        nir_jump_instr *instr);
>>     fs_reg get_nir_src(nir_src src);
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index ff5ac9c..61058b2 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -444,7 +444,6 @@ void
>>  fs_visitor::nir_emit_instr(nir_instr *instr)
>>  {
>>     const fs_builder abld = bld.annotate(NULL, instr);
>> -   this->base_ir = instr;
>
> I'd make this...
>
>>
>>     switch (instr->type) {
>>     case nir_instr_type_alu:
>> @@ -456,7 +455,7 @@ fs_visitor::nir_emit_instr(nir_instr *instr)
>>        break;
>>
>>     case nir_instr_type_tex:
>> -      nir_emit_texture(nir_instr_as_tex(instr));
>> +      nir_emit_texture(abld, nir_instr_as_tex(instr));
>>        break;
>>
>>     case nir_instr_type_load_const:
>> @@ -472,8 +471,6 @@ fs_visitor::nir_emit_instr(nir_instr *instr)
>>     default:
>>        unreachable("unknown instruction type");
>>     }
>> -
>> -   this->base_ir = NULL;
>
> ... and this a separate commit. Otherwise there's an implicit ordering
> to the patches to brw_fs_nir.cpp.

There is also a similar ordering dependency between PATCH 31 and the
following because of the "abld" local defined in that file.  Should I
also split that definition into a separate commit?  It seems silly to
split a one-line change defining a local variable from the first commit
that uses it, and a two-line change removing a dead assignment from the
commit that makes the assignment dead -- Unless you have some reason to
land these patches in a different order?
-------------- 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/f19a1845/attachment.sig>