<!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 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 04:12:44 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 Thu, 2019-01-17 at 14:14 -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 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">We are now using these bits, so don't assert that they are not set, just<br> avoid compaction in that case.<br> <br> Reviewed-by: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com" target="_blank">topi.pohjolainen@intel.com</a>><br> ---<br> src/intel/compiler/brw_eu_compact.c | 5 ++++-<br> 1 file changed, 4 insertions(+), 1 deletion(-)<br> <br> diff --git a/src/intel/compiler/brw_eu_compact.c b/src/intel/compiler/brw_eu_compact.c<br> index ae14ef10ec0..20fed254331 100644<br> --- a/src/intel/compiler/brw_eu_compact.c<br> +++ b/src/intel/compiler/brw_eu_compact.c<br> @@ -928,8 +928,11 @@ has_3src_unmapped_bits(const struct gen_device_info *devinfo,<br> assert(!brw_inst_bits(src, 127, 126) &&<br> !brw_inst_bits(src, 105, 105) &&<br> !brw_inst_bits(src, 84, 84) &&<br> - !brw_inst_bits(src, 36, 35) &&<br> !brw_inst_bits(src, 7, 7));<br> +<br> + /* Src1Type and Src2Type, used for mixed-precision floating point */<br> + if (brw_inst_bits(src, 36, 35))<br> + return true;<br></blockquote><div><br></div><div>You're only doing this in the broadwell case. What about SKL+ and CHV? Can we compact mixed-precision stuff there? Looks like maybe we can but there should be at least something in the commit message about that.<br></div></div></div></blockquote><div><br></div><div>In these platforms compaction is possible in some cases and set_3src_control_index() takes</div><div>care of this by including these bits in a tablre lookup for accepted combinations. I can add this</div><div>to the commit message:</div><div><br></div><div>"We are now using these bits, so don't assert that they are not set. In gen8, if these bits are set</div><div>compaction is not possible. On gen9 and CHV platforms set_3src_control_index() checks these</div><div>bits (and others) against a table to validate if the particular bit combination is eligible for</div><div>compaction or not."</div><div><br></div><div>With that said, if I am reading this correctly, it looks like all entries in gen8_3src_control_index_table that allow compaction require these bits to be zero at present, so I guess that right now we could also just extend the check I am adding here for BDW to other platforms, however, I guess that relying on the array with accepted bit combinations for compaction is more reliable should any future platforms allow for more combinations.</div></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">Sounds good. With that commit message update, r-b me.</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"><div dir="ltr"><div class="gmail_quote"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"> }<br> <br> return false;<br> </blockquote></div></div></blockquote></blockquote> </div> </div>  </div><div dir="auto"><br></div> </div></body> </html>