On 06/30/2016 03:33 PM, Francisco Jerez wrote:
> Matt Turner <matts...@gmail.com> writes:
>
>> On Wed, Jun 29, 2016 at 2:04 PM, Ian Romanick <i...@freedesktop.org> wrote:
>>> From: Ian Romanick <ian.d.roman...@intel.com>
>>>
>>> This uses one less instruction.
>>
>> Add FBH to the list of stupid instructions.
>>
>>> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
>>> ---
>>> src/mesa/drivers/dri/i965/brw_fs.h | 4 ++++
>>> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 3 +++
>>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 26
>>> +++++++++++++++++++++++-
>>> src/mesa/drivers/dri/i965/brw_vec4.h | 4 ++++
>>> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 3 +++
>>> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 22 ++++++++++++++++++++
>>> 6 files changed, 61 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
>>> b/src/mesa/drivers/dri/i965/brw_fs.h
>>> index 4237197..22ce092 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>>> @@ -237,6 +237,10 @@ public:
>>> nir_tex_instr *instr);
>>> void nir_emit_jump(const brw::fs_builder &bld,
>>> nir_jump_instr *instr);
>>> + void nir_emit_find_msb_using_lzd(const brw::fs_builder &bld,
>>> + const fs_reg &result,
>>> + const fs_reg &src,
>>> + bool is_signed);
>>> fs_reg get_nir_src(const nir_src &src);
>>> fs_reg get_nir_src_imm(const nir_src &src);
>>> fs_reg get_nir_dest(const nir_dest &dest);
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>>> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>>> index d25d26a..bda4a26 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>>> @@ -1761,6 +1761,9 @@ fs_generator::generate_code(const cfg_t *cfg, int
>>> dispatch_width)
>>> /* FBL only supports UD type for dst. */
>>> brw_FBL(p, retype(dst, BRW_REGISTER_TYPE_UD), src[0]);
>>> break;
>>> + case BRW_OPCODE_LZD:
>>> + brw_LZD(p, dst, src[0]);
>>> + break;
>>> case BRW_OPCODE_CBIT:
>>> assert(devinfo->gen >= 7);
>>> /* CBIT only supports UD type for dst. */
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> index b3f5dfd..f15bf3e 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> @@ -617,6 +617,25 @@ fs_visitor::optimize_frontfacing_ternary(nir_alu_instr
>>> *instr,
>>> }
>>>
>>> void
>>> +fs_visitor::nir_emit_find_msb_using_lzd(const fs_builder &bld,
>>> + const fs_reg &result,
>>> + const fs_reg &src,
>>> + bool is_signed)
>>> +{
>>> + fs_inst *inst;
>>> +
>>> + bld.LZD(retype(result, BRW_REGISTER_TYPE_UD), src);
>>> +
>>> + /* LZD counts from the MSB side, while GLSL's findMSB() wants the count
>>> + * from the LSB side. Subtract the result from 31 to convert the MSB
>>> + * count into an LSB count. If no bits are set, LZD will return 32.
>>> + * 31-32 = -1, which is exactly what findMSB() is supposed to return.
>>> + */
>>> + inst = bld.ADD(result, retype(result, BRW_REGISTER_TYPE_D),
>>> brw_imm_d(31));
>>> + inst->src[0].negate = true;
>>> +}
>>
>> I'd personally be inclined to just inline these functions. I know they
>> grow somewhat in the next patches... whatever your preference is.
>
> It seems to grow quite a bit in PATCH 16, and it's used in multiple
> places, right? How about we keep it factored out but make it a
> stand-alone function instead of a visitor method? It doesn't look like
> it uses *any* internal or external data structures of fs_visitor, it
> doesn't even dereference the 'this' pointer at all AFAICT, so you could
> likely improve encapsulation somewhat by making it a static function
> local to the brw_*_nir.cpp source files.Okay... two problems: In the vec4 visitor, all of the emit() functions are in visitor, so this new function has to stay there. In the fs visitor, the next patch introduces a call to fs_visitor::vgrf. See previous problem. :( >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
