On Wed, Dec 2, 2015 at 7:54 AM, Connor Abbott <cwabbo...@gmail.com> wrote:
> On Wed, Dec 2, 2015 at 3:26 AM, Kenneth Graunke <kenn...@whitecape.org> wrote:
>> Normally, get_nir_src() won't return an immediate value - it moves it to
>> a temporary VGRF. There are consumers of get_nir_src() that rely on
>> this, and are unprepared to handle immediate values.
>>
>> This patch introduces new helpers which return immediates for single
>> component constant values, and VGRFs for other values.
>
> Just curious, what are you going to use this for? Constant propagation
> should always be smart enough to fold constants into the instruction,
> since the constants are SSA values that turn into registers which are
> only written once.
>
>>
>> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
>> ---
>> src/mesa/drivers/dri/i965/brw_fs.h | 1 +
>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 11 +++++++++++
>> src/mesa/drivers/dri/i965/brw_vec4.h | 1 +
>> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 7 +++++++
>> 4 files changed, 20 insertions(+)
>>
>> I'm using this in my tessellation branch, but I noticed Jason open-coding
>> this pattern in a bunch of his recent patches, so I thought I'd send it out
>> a bit early in case he wanted to use it, too.
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
>> b/src/mesa/drivers/dri/i965/brw_fs.h
>> index bca4589..ce3bdef 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -256,6 +256,7 @@ public:
>> 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_src_imm(nir_src src);
>> fs_reg get_nir_dest(nir_dest dest);
>> fs_reg get_nir_image_deref(const nir_deref_var *deref);
>> 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 9b50e4e..fc71a0f 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -1136,6 +1136,17 @@ fs_visitor::get_nir_src(nir_src src)
>> return retype(reg, BRW_REGISTER_TYPE_D);
>> }
>>
>> +/**
>> + * Return an IMM for constants; otherwise call get_nir_src() as normal.
>> + */
>> +fs_reg
>> +fs_visitor::get_nir_src_imm(nir_src src)
>> +{
>> + nir_const_value *val = nir_src_as_const_value(src);
>> + return val && src.ssa->num_components == 1 ? fs_reg(brw_imm_d(val->i[0]))
>> + : get_nir_src(src);
>> +}
>> +
>> fs_reg
>> fs_visitor::get_nir_dest(nir_dest dest)
>> {
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
>> b/src/mesa/drivers/dri/i965/brw_vec4.h
>> index 25b1139..0150bb9 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
>> @@ -341,6 +341,7 @@ public:
>> unsigned num_components = 4);
>> src_reg get_nir_src(nir_src src,
>> unsigned num_components = 4);
>> + src_reg get_nir_src_imm(nir_src src, enum brw_reg_type type);
>>
>> virtual dst_reg *make_reg_for_system_value(int location,
>> const glsl_type *type) = 0;
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> index 4aed60e..ffd480a 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -348,6 +348,13 @@ vec4_visitor::get_nir_src(nir_src src, unsigned
>> num_components)
>> return get_nir_src(src, nir_type_int, num_components);
>> }
>>
>> +src_reg
>> +vec4_visitor::get_nir_src_imm(nir_src src, enum brw_reg_type type)
>> +{
>> + nir_const_value *val = nir_src_as_const_value(src);
>> + return val ? retype(src_reg(val->u[0]), type) : get_nir_src(src, type,
>> 1);
>> +}
>
> Don't you need to check here if the constant value has one component
> like you do in FS? Also, this isn't handling VF immediates, where we
> can pack something larger than one component into an immediate...
> maybe at least worth a comment?
I think that's fine. Optimizing with VF immediates is hard, so we have
opt_vector_float() that gathers things into VFs after the main
optimization loop.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
