<html dir="ltr"><head></head><body style="text-align:left; direction:ltr;"><div>Yes, I mentioned this in another of my replies. I think we probably want a combination of these things. For some restrictions like the cases where we need to split a conversion in two instructions going through an intermediate type I think a NIR lowering would be good, but then I think we might also want to create one or two specialized conversion helpers in the backend that deal with specific groups of situations such as maybe conversions to/from 64-bit or to/from lower bit-sizes, or maybe to/from integer, etc I need to think a bit more about what would be a good strategy, but in any case, I share your view of the situation and I agree we should do something about it.</div><div><br></div><div>Iago</div><div><br></div><div>On Fri, 2018-12-07 at 13:53 -0600, Jason Ekstrand wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr">We are starting to get a *lot* of special cases in the conversion code. I'm not sure what the best thing to do is. Maybe some master conversion function that just does it all? Maybe some NIR lowering? In any case, I think we can do better than the pile of special cases we are starting to accumulate.<br></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Dec 4, 2018 at 1:18 AM Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote:<br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">Section Register Region Restriction of the 3D Media GPGPU chapter states:<br>
<br>
"Conversion between Integer and HF (Half Float) must be DWord<br>
aligned and strided by a DWord on the destination."<br>
<br>
The same restriction shows up in all hardware platforms that support<br>
half-float, however, empirical testing suggests that only atom<br>
platforms are affected.<br>
---<br>
src/intel/compiler/brw_fs_nir.cpp | 41 +++++++++++++++++++++++++++++--<br>
1 file changed, 39 insertions(+), 2 deletions(-)<br>
<br>
diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp<br>
index 3f98c6a4474..db3a8812ae3 100644<br>
--- a/src/intel/compiler/brw_fs_nir.cpp<br>
+++ b/src/intel/compiler/brw_fs_nir.cpp<br>
@@ -917,6 +917,25 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)<br>
inst->saturate = instr->dest.saturate;<br>
break;<br>
}<br>
+<br>
+ /* CHV PRM, 3D Media GPGPU Engine, Register Region Restrictions,<br>
+ * Special Restrictions:<br>
+ *<br>
+ * "Conversion between Integer and HF (Half Float) must be DWord<br>
+ * aligned and strided by a DWord on the destination."<br>
+ *<br>
+ * The same restriction is listed for other hardware platforms, however,<br>
+ * empirical testing suggests that only atom platforms are affected.<br>
+ */<br>
+ if ((devinfo->is_cherryview || gen_device_info_is_9lp(devinfo)) &&<br>
+ nir_dest_bit_size(instr->dest.dest) == 16) {<br>
+ assert(result.type == BRW_REGISTER_TYPE_HF);<br>
+ fs_reg tmp =<br>
+ horiz_stride(retype(bld.vgrf(BRW_REGISTER_TYPE_F, 1), result.type), 2);<br>
+ bld.MOV(tmp, op[0]);<br>
+ op[0] = tmp;<br>
+ }<br>
+<br>
inst = bld.MOV(result, op[0]);<br>
inst->saturate = instr->dest.saturate;<br>
break;<br>
@@ -939,11 +958,29 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)<br>
}<br>
/* Fallthrough */<br>
<br>
+ case nir_op_f2i16:<br>
+ case nir_op_f2u16:<br>
+ /* CHV PRM, 3D Media GPGPU Engine, Register Region Restrictions,<br>
+ * Special Restrictions:<br>
+ *<br>
+ * "Conversion between Integer and HF (Half Float) must be DWord<br>
+ * aligned and strided by a DWord on the destination."<br>
+ *<br>
+ * The same restriction is listed for other hardware platforms, however,<br>
+ * empirical testing suggests that only atom platforms are affected.<br>
+ */<br>
+ if ((devinfo->is_cherryview || gen_device_info_is_9lp(devinfo)) &&<br>
+ nir_src_bit_size(instr->src[0].src) == 16) {<br>
+ fs_reg tmp =<br>
+ horiz_stride(retype(bld.vgrf(BRW_REGISTER_TYPE_D, 1), result.type), 2);<br>
+ bld.MOV(tmp, op[0]);<br>
+ op[0] = tmp;<br>
+ }<br>
+ /* Fallthrough */<br>
+<br>
case nir_op_f2f32:<br>
case nir_op_f2i32:<br>
case nir_op_f2u32:<br>
- case nir_op_f2i16:<br>
- case nir_op_f2u16:<br>
case nir_op_i2i32:<br>
case nir_op_u2u32:<br>
case nir_op_i2i16:<br>
</blockquote></div></blockquote></body></html>