[Mesa-dev] [PATCH 12/78] i965/nir/vec4: Add nir_get_dst() and nir_get_src() methods
Eduardo Lima Mitev
elima at igalia.com
Thu Jul 2 02:54:24 PDT 2015
On 06/30/2015 06:26 PM, Jason Ekstrand wrote:
> On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
>> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>>
>> These methods are essential for the implementation of the NIR->vec4 pass. They
>> work similar to their fs_nir counter-parts.
>>
>> When processing instructions, these methods are invoked to resolve the
>> brw registers (source or destination) corresponding to the NIR sources
>> or destination. It builds a map of NIR register index to brw register for
>> all registers locally allocated in a block.
>>
>> In the case of get_nir_src(), it also builds immediate registers on-the-fly
>> when queried for a SSA source which at this point can only correspond to
>> constant values.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
>> ---
>> src/mesa/drivers/dri/i965/brw_vec4.h | 7 ++
>> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 102 +++++++++++++++++++++++++++++
>> 2 files changed, 109 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
>> index d837d90..a0f5935 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
>> @@ -411,6 +411,13 @@ public:
>> virtual void nir_emit_jump(nir_jump_instr *instr);
>> virtual void nir_emit_texture(nir_tex_instr *instr);
>>
>> + dst_reg get_nir_dest(nir_dest dest, enum brw_reg_type type);
>> + dst_reg get_nir_dest(nir_dest dest, nir_alu_type type);
>> + dst_reg get_nir_dest(nir_dest dest);
>> + src_reg get_nir_src(nir_src src, enum brw_reg_type type);
>> + src_reg get_nir_src(nir_src src, nir_alu_type type);
>> + src_reg get_nir_src(nir_src src);
>> +
>> 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 36c9dc0..1ec75ee 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
>> @@ -365,6 +365,108 @@ vec4_visitor::nir_emit_instr(nir_instr *instr)
>> }
>> }
>>
>> +static dst_reg
>> +dst_reg_for_nir_reg(vec4_visitor *v, nir_register *nir_reg,
>> + unsigned base_offset, nir_src *indirect)
>> +{
>> + dst_reg reg;
>> +
>> + reg = v->nir_locals[nir_reg->index];
>> +
>> + reg = offset(reg, base_offset * nir_reg->num_components);
>> + if (indirect) {
>> + int multiplier = nir_reg->num_components;
>> +
>> + reg.reladdr = new(v->mem_ctx) src_reg(dst_reg(GRF, v->alloc.allocate(1)));
>> + v->emit(v->MUL(dst_reg(*reg.reladdr),
>> + retype(v->get_nir_src(*indirect), BRW_REGISTER_TYPE_D),
>> + src_reg(multiplier)));
>
> I'm not sure this is correct. We need this in the FS backend because
> each register is a scalar value. In vec4, each array element should
> correspond to a single vec4 register so you shouldn't need to
> multiply. Actually, I think it's currently dead code in the backend
> because we lower it all away, so it may not be correct at all.
>
You are right, this multiply is not needed. I already removed it and
verified it. This will be changed in v2.
>> + }
>> +
>> + return reg;
>> +}
>> +
>> +dst_reg
>> +vec4_visitor::get_nir_dest(nir_dest dest)
>> +{
>> + return dst_reg_for_nir_reg(this, dest.reg.reg, dest.reg.base_offset,
>> + dest.reg.indirect);
>> +}
>> +
>> +dst_reg
>> +vec4_visitor::get_nir_dest(nir_dest dest, enum brw_reg_type type)
>> +{
>> + dst_reg reg = get_nir_dest(dest);
>> + return retype(reg, type);
>> +}
>> +
>> +dst_reg
>> +vec4_visitor::get_nir_dest(nir_dest dest, nir_alu_type type)
>> +{
>> + dst_reg reg = get_nir_dest(dest);
>> + return retype(reg, brw_type_for_nir_type(type));
>> +}
>> +
>> +src_reg
>> +vec4_visitor::get_nir_src(nir_src src, enum brw_reg_type type)
>> +{
>> + dst_reg reg;
>> +
>> + if (src.is_ssa) {
>> + assert(src.ssa->parent_instr->type == nir_instr_type_load_const);
>> + nir_load_const_instr *load = nir_instr_as_load_const(src.ssa->parent_instr);
>> +
>> + reg = dst_reg(GRF, alloc.allocate(src.ssa->num_components));
>
> It's vec4, you only need one register, not num_components-many.
>
Right. This comes from our initial misconception of how the units were
given in vec4 vs. scalar. I have changed that everywhere in the backend
already, to allocate exactly 1 unit.
>> + reg = retype(reg, type);
>> +
>> + /* @FIXME: while this is what fs_nir does, we can do this better in the VS
>> + * stage because we can emit vector operations and save some MOVs in
>> + * cases where the constants are representable in 8 bits.
>> + * So by now, we emit a MOV for each component.
>> + */
>> + for (unsigned i = 0; i < src.ssa->num_components; ++i) {
>> + reg.writemask = 1 << i;
>> +
>> + switch (reg.type) {
>> + case BRW_REGISTER_TYPE_F:
>> + emit(MOV(reg, src_reg(load->value.f[i])));
>> + break;
>> + case BRW_REGISTER_TYPE_D:
>> + emit(MOV(reg, src_reg(load->value.i[i])));
>> + break;
>> + case BRW_REGISTER_TYPE_UD:
>> + emit(MOV(reg, src_reg(load->value.u[i])));
>> + break;
>> + default:
>> + unreachable("invalid register type");
>> + }
>> + }
>> +
>> + /* Set final writemask */
>> + reg.writemask = brw_writemask_for_size(src.ssa->num_components);
>> + }
>> + else {
>> + reg = dst_reg_for_nir_reg(this, src.reg.reg, src.reg.base_offset,
>> + src.reg.indirect);
>> + reg = retype(reg, type);
>
> Why do you support indirects for destinations but not here?
>
It is indeed supported. Notice I'm calling dst_reg_for_nir_reg(), which
has the indirect handling. get_nir_dst() also uses it.
>> + }
>> +
>> + return src_reg(reg);
>> +}
>> +
>> +src_reg
>> +vec4_visitor::get_nir_src(nir_src src, nir_alu_type type)
>> +{
>> + return get_nir_src(src, brw_type_for_nir_type(type));
>> +}
>> +
>> +src_reg
>> +vec4_visitor::get_nir_src(nir_src src)
>> +{
>> + /* if type is not specified, default to signed int */
>> + return get_nir_src(src, nir_type_int);
>> +}
>> +
>> void
>> vec4_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
>> {
>> --
>> 2.1.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list