On 08/07/16 00:27, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes:
>
>> From: Iago Toral Quiroga <ito...@igalia.com>
>>
>> Gen7 hardware does not support double immediates so these need
>> to be moved in 32-bit chunks to a regular vgrf instead. Instead
>> of doing this every time we need to create a DF immediate,
>> create a helper function that does the right thing depending
>> on the hardware generation.
>> ---
>> src/mesa/drivers/dri/i965/brw_fs.h | 2 ++
>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 43
>> ++++++++++++++++++++++++++++++++
>> 2 files changed, 45 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
>> b/src/mesa/drivers/dri/i965/brw_fs.h
>> index 4237197..dd7ce7d 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
>> @@ -167,6 +167,8 @@ public:
>> bool lower_simd_width();
>> bool opt_combine_constants();
>>
>> + fs_reg setup_imm_df(double v);
>> +
>> void emit_dummy_fs();
>> void emit_repclear_shader();
>> fs_reg *emit_fragcoord_interpolation();
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index b3f5dfd..268c847 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -616,6 +616,49 @@ fs_visitor::optimize_frontfacing_ternary(nir_alu_instr
>> *instr,
>> return true;
>> }
>>
>> +fs_reg
>> +fs_visitor::setup_imm_df(double v)
>
> Mainly nitpicking here, but because this function only needs an i965 IR
> builder and doesn't otherwise care about the fs_visitor class, it would
> make more sense for it to be a stand-alone function independent from
> fs_visitor taking an fs_builder as argument instead.
>
>> +{
>> + assert(devinfo->gen >= 7);
>> +
>> + if (devinfo->gen >= 8)
>> + return brw_imm_df(v);
>> +
>> + /* gen7 does not support DF immediates, so we generate a 64-bit constant
>> by
>> + * writing the low 32-bit of the constant to suboffset 0 of a VGRF and
>> + * the high 32-bit to suboffset 4 and then applying a stride of 0.
>> + *
>> + * Alternatively, we could also produce a normal VGRF (without stride 0)
>> + * by writing to all the channels in the VGRF, however, that would hit
>> the
>> + * gen7 bug where we have to split writes that span more than 1 register
>> + * into instructions with a width of 4 (otherwise the write to the second
>> + * register written runs into an execmask hardware bug) which isn't very
>> + * nice.
>> + */
>> + union {
>> + double d;
>> + struct {
>> + uint32_t i1;
>> + uint32_t i2;
>> + };
>> + } di;
>> +
>> + di.d = v;
>> +
>
> You can declare a scalar builder here for convenience like:
>
> | const fs_builder ubld = bld.exec_all().group(1, 0);
>
> then use it below instead of 'bld' so you can get rid of the six inst
> field assignments.
>
>> + fs_reg tmp = vgrf(glsl_type::uint_type);
>
> On e.g. SIMD32 mode this will allocate 32 components worth of registers
> even though you only need two. Once you have a scalar builder at hand
> you can do it as follows instead:
>
> | const fs_reg tmp = ubld.vgrf(BRW_REGISTER_TYPE_UD, 2);
>
> Other than that looks okay to me.
>
OK, thanks for the suggestions! I will implement them.
Sam
>> + fs_inst *inst = bld.MOV(tmp, brw_imm_ud(di.i1));
>> + inst->force_writemask_all = true;
>> + inst->exec_size = 1;
>> + inst->regs_written = 1;
>> +
>> + inst = bld.MOV(horiz_offset(tmp, 1), brw_imm_ud(di.i2));
>> + inst->force_writemask_all = true;
>> + inst->exec_size = 1;
>> + inst->regs_written = 1;
>> +
>> + return component(retype(tmp, BRW_REGISTER_TYPE_DF), 0);
>> +}
>> +
>> void
>> fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)
>> {
>> --
>> 2.7.4
>>
>> _______________________________________________
>> 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
