<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Oct 12, 2017 at 11:38 AM, Jose Maria Casanova Crespo <span dir="ltr"><<a href="mailto:jmcasanova@igalia.com" target="_blank">jmcasanova@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Added on do_untyped_vector_read, that is used on the following<br>
intrinsics:<br>
* nir_intrinsic_load_shared<br>
* nir_intrinsic_load_ubo<br>
* nir_intrinsic_load_ssbo<br>
<br>
v2: Removed use of stride = 2 on 16-bit sources (Jason Ekstrand)<br>
---<br>
src/intel/compiler/brw_fs_nir.<wbr>cpp | 24 ++++++++++++++++++++----<br>
1 file changed, 20 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/src/intel/compiler/brw_fs_<wbr>nir.cpp b/src/intel/compiler/brw_fs_<wbr>nir.cpp<br>
index 13c16fc912..d2f2e17b70 100644<br>
--- a/src/intel/compiler/brw_fs_<wbr>nir.cpp<br>
+++ b/src/intel/compiler/brw_fs_<wbr>nir.cpp<br>
@@ -2268,6 +2268,18 @@ do_untyped_vector_read(const fs_builder &bld,<br>
<br>
bld.ADD(read_offset, read_offset, brw_imm_ud(16));<br>
}<br>
+ } else if (type_sz(dest.type) == 2) {<br>
+ for (unsigned i = 0; i < num_components; i++) {<br>
+ fs_reg base_offset = bld.vgrf(BRW_REGISTER_TYPE_UD)<wbr>;<br>
+ bld.ADD(base_offset,<br>
+ offset_reg,<br>
+ brw_imm_ud(i * type_sz(dest.type)));<br>
+ fs_reg read_reg = emit_byte_scattered_read(bld, surf_index, base_offset,<br>
+ 1 /* dims */,<br>
+ 1,<br>
+ BRW_PREDICATE_NONE);<br>
+ bld.MOV(offset(dest,bld,i), subscript(read_reg, dest.type, 0));<br>
+ }<br></blockquote><div><br></div><div>This looks fine though I'm not actually sure byte scattered reads are actually going to buy us anything over doing a DWORD read and unpacking the 16-bit values from the result.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
} else {<br>
unreachable("Unsupported type");<br>
}<br>
@@ -3911,10 +3923,14 @@ fs_visitor::nir_emit_<wbr>intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr<br>
if (const_offset == NULL) {<br>
fs_reg base_offset = retype(get_nir_src(instr->src[<wbr>1]),<br>
BRW_REGISTER_TYPE_UD);<br>
-<br>
- for (int i = 0; i < instr->num_components; i++)<br>
- VARYING_PULL_CONSTANT_LOAD(<wbr>bld, offset(dest, bld, i), surf_index,<br>
- base_offset, i * type_sz(dest.type));<br>
+ if (type_sz(dest.type) == 2) {<br>
+ do_untyped_vector_read(bld, dest, surf_index, base_offset,<br>
+ instr->num_components);<br>
+ } else {<br>
+ for (int i = 0; i < instr->num_components; i++)<br>
+ VARYING_PULL_CONSTANT_LOAD(<wbr>bld, offset(dest, bld, i), surf_index,<br>
+ base_offset, i * type_sz(dest.type));<br>
+ }<br></blockquote><div><br></div><div>I don't like lumping this change in with the other and I'm not sure I like the change at all. This switches us from using the sampler cache over to using the data cache silently based on the component size. I would rather we either switch 100% over to the byte/oword scattered messages at some hardware generation, or just keep with the sampler operation we're doing today. From my reading of the SKL docs, it looks like we can't actually use the constant cache with the byte scattered messages but we could with the DWORD scattered messages which may be a good change in future (though Curro may have some reason we don't want to do that). Unless we have a very good reason why we need to use the byte messages, let's use something that goes through the constant or sampler cache. This is for UBO constant data pulls so there's no real problem with reading too much data.<br></div><div><br></div><div>I think the thing to do in the short term is just do a VARYING_PULL_CONSTANT_LOAD for each pair of components and then split them out into 16-bit chunks. Also, that should be it's own patch.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
} else {<br>
/* Even if we are loading doubles, a pull constant load will load<br>
* a 32-bit vec4, so should only reserve vgrf space for that. If we<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.13.6<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div>