<div dir="ltr">On 2 December 2013 11:31, Francisco Jerez <span dir="ltr"><<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+/**<br>
+ * Get either of the 8-component halves of a 16-component register.<br>
+ */<br>
+static inline fs_reg<br>
+half(const fs_reg &reg, unsigned idx)<br>
+{<br>
+   assert(idx == 0 || (reg.file != HW_REG && reg.file != IMM));<br>
+   return byte_offset(reg, 8 * idx * reg.stride * type_sz(reg.type));<br>
+}<br>
+<br></blockquote><div><br></div><div>I'd like to see a comment explaining that it's safe to call this function on a register with 32 bits per component, since brw_reg.h's byte_offset() handles byte offsets greater than 32 by incrementing the register number.<br>
<br></div><div>Also, for sanity's sake, it would be nice for this function to include the assertion:<br><br></div><div>   assert(idx < 2);<br><br></div><div>With those changes, this patch is:<br><br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br>
</div></div><br></div></div>