On Tue, 2018-12-04 at 18:10 +0200, Pohjolainen, Topi wrote: > On Tue, Dec 04, 2018 at 02:33:25PM +0200, Pohjolainen, Topi wrote: > > On Tue, Dec 04, 2018 at 08:16:34AM +0100, Iago Toral Quiroga wrote: > > > Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com> > > > --- > > > src/intel/compiler/brw_fs_nir.cpp | 41 > > > +++++++++++++++++++++++++++++++ > > > 1 file changed, 41 insertions(+) > > > > > > diff --git a/src/intel/compiler/brw_fs_nir.cpp > > > b/src/intel/compiler/brw_fs_nir.cpp > > > index 6eb68794f58..7294f49ddc0 100644 > > > --- a/src/intel/compiler/brw_fs_nir.cpp > > > +++ b/src/intel/compiler/brw_fs_nir.cpp > > > @@ -796,6 +796,47 @@ fs_visitor::nir_emit_alu(const fs_builder > > > &bld, nir_alu_instr *instr) > > > case nir_op_f2f64: > > > case nir_op_f2i64: > > > case nir_op_f2u64: > > > + /* BDW PRM, vol02, Command Reference Instructions, mov - > > > MOVE: > > > + * > > > + * "There is no direct conversion from HF to DF or DF to > > > HF. > > > + * Use two instructions and F (Float) as an > > > intermediate type. > > > + * > > > + * There is no direct conversion from HF to Q/UQ or > > > Q/UQ to HF. > > > + * Use two instructions and F (Float) or a word integer > > > type > > > + * or a DWord integer type as an intermediate type." > > > + */ > > > + if (nir_src_bit_size(instr->src[0].src) == 16) { > > > + fs_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_F, 1); > > > + inst = bld.MOV(tmp, op[0]); > > > + inst->saturate = instr->dest.saturate; > > > + op[0] = tmp; > > > + } > > > + > > > + /* CHV PRM, vol07, 3D Media GPGPU Engine, Register Region > > > Restrictions: > > > + * > > > + * "When source or destination is 64b (...), regioning > > > in Align1 > > > + * must follow these rules: > > > + * > > > + * 1. Source and destination horizontal stride must be > > > aligned to > > > + * the same qword. > > > + * (...)" > > > + * > > > + * This means that conversions from bit-sizes smaller than > > > 64-bit to > > > + * 64-bit need to have the source data elements aligned to > > > 64-bit. > > > + * This restriction does not apply to BDW and later. > > > + */ > > > + if (type_sz(result.type) == 8 && type_sz(op[0].type) < 8 > > > && > > > + (devinfo->is_cherryview || > > > gen_device_info_is_9lp(devinfo))) { > > > + fs_reg tmp = bld.vgrf(result.type, 1); > > > + tmp = subscript(tmp, op[0].type, 0); > > > + inst = bld.MOV(tmp, op[0]); > > > + op[0] = tmp; > > > + } > > > > For this second part we seem to have similar logic further down > > after > > "nir_op_u2u64" (not visible here) in master? Would it be possible > > to fallthru > > from here and re-use that? > > And after reading it more carefully myself it looks that this is > actually > cleaner. > > I noticed that in the nir_op_u2u64 case the destination and source > sizes are > checked using: > > if (nir_dest_bit_size(instr->dest.dest) == 64 && > nir_src_bit_size(instr->src[0].src) < 64 && > ... > > Should we use the same here for consistency?
Right above this we can rewrite op[0] with a temporary that would be different from instr->src[0].src so we can't check the nir sources any more. Also, by the end of the series, when we incorporate 8-bit conversions there will be more cases like this that we need to account for and we end up rewriting the u2u64 case to this style as well. Iago > > > > > + > > > + inst = bld.MOV(result, op[0]); > > > + inst->saturate = instr->dest.saturate; > > > + break; > > > + > > > case nir_op_i2f64: > > > case nir_op_i2i64: > > > case nir_op_u2f64: > > > -- > > > 2.17.1 > > > > > > _______________________________________________ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev