<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"> <html> <body> <div dir="auto"> <div dir="auto"><br></div><div dir="auto"><br></div> <div id="aqm-original" style="color: black;">  <div style="text-align:left; direction:ltr;" class="aqm-original-body"><div style="color: black;"> <p style="color: black; font-size: 10pt; font-family: sans-serif; margin: 8pt 0;">On January 18, 2019 06:23:35 Iago Toral <itoral@igalia.com> wrote:</p> <blockquote type="cite" class="gmail_quote" style="margin: 0 0 0 0.75ex; border-left: 1px solid #808080; padding-left: 0.75ex;"> <div>On Fri, 2019-01-18 at 12:13 +0100, Iago Toral wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div>On Thu, 2019-01-17 at 17:12 -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"><div>This patch doesn't really do what the commit message says. What it really does is implement float -> 8-bit converions for *any* size float.<br></div><div><br></div><div class="gmail_quote"><div dir="ltr">On Tue, Jan 15, 2019 at 7:55 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">These are not directly supported in hardware and brw_nir_lower_conversions<br> should have taken care of that before we get here. Also, while we are<br> at it, make sure 64-bit integer to 8-bit are also properly split by<br> the same lowering pass, since they have the same hardware restrictions.<br></blockquote><div><br></div><div>Now that we have a lowering pass, having separate cases just so one of them can assert seems silly. If anything, we should just do</div><div><br></div><div>if (result.type == BRW_REGISTER_TYPE_B ||</div><div> result.type == BRW_REGISTER_TYPE_UB ||</div><div> result.type == BRW_REGISTER_TYPE_HF)</div><div> assert(type_sz(op[0].type) < 8) /* brw_nir_lower_conversions */</div><div><br></div><div>and have it all in one big case. The only special case we need is for booleans where we need to negate them and fall through.</div></div></div></blockquote><div><br></div><div>There are more cases, since the inverse conversions of these are not supported either. I guess I'll just add this as well:</div><div><br></div><div>if (op[0].type == BRW_REGISTER_TYPE_B ||</div><div> op[0].type == BRW_REGISTER_TYPE_UB ||</div><div> op[0].type == BRW_REGISTER_TYPE_HF)</div><div> assert(type_sz(result.type) < 8); /* brw_nir_lower_conversions */</div></blockquote><div><br></div><div>Oh, and there is also the rounding opcodes for f16 destinations (plus the big comment about brw_F32TO16 that comes with their fallthrough)... I think we might want to keep the three f2f16* opcodes as a separate block and do as you suggest for the remaining ones if that is okay.</div></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">That's fine as they actually do do something different.</div><div dir="auto"><br></div><div dir="auto">-Jason</div><div dir="auto"><br></div><div id="aqm-original" style="color: black;" dir="auto"><div style="text-align:left; direction:ltr;" class="aqm-original-body"><div style="color: black;"><blockquote type="cite" class="gmail_quote" style="margin: 0 0 0 0.75ex; border-left: 1px solid #808080; padding-left: 0.75ex;"><div></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>--Jason<br></div><div> </div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"> ---<br> src/intel/compiler/brw_fs_nir.cpp | 6 ++++--<br> 1 file changed, 4 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 cf546b8ff09..e454578d99b 100644<br> --- a/src/intel/compiler/brw_fs_nir.cpp<br> +++ b/src/intel/compiler/brw_fs_nir.cpp<br> @@ -786,6 +786,10 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)<br> case nir_op_f2f16:<br> case nir_op_i2f16:<br> case nir_op_u2f16:<br> + case nir_op_i2i8:<br> + case nir_op_u2u8:<br> + case nir_op_f2i8:<br> + case nir_op_f2u8:<br> assert(type_sz(op[0].type) < 8); /* brw_nir_lower_conversions */<br> inst = bld.MOV(result, op[0]);<br> inst->saturate = instr->dest.saturate;<br> @@ -824,8 +828,6 @@ fs_visitor::nir_emit_alu(const fs_builder &bld, nir_alu_instr *instr)<br> case nir_op_u2u32:<br> case nir_op_i2i16:<br> case nir_op_u2u16:<br> - case nir_op_i2i8:<br> - case nir_op_u2u8:<br> inst = bld.MOV(result, op[0]);<br> inst->saturate = instr->dest.saturate;<br> break;<br> </blockquote></div></div></blockquote> </blockquote></blockquote> </div> </div>  </div><div dir="auto"><br></div> </div></body> </html>