<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Feb 23, 2018 at 1:26 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">Restrict the use of untyped_surface_write with 16-bit pairs in<br>
ssbo to the cases where we can guarantee that offset is multiple<br>
of 4.<br>
<br>
Taking into account that VK_KHR_relaxed_block_layout is available<br>
in ANV we can only guarantee that when we have a constant offset<br>
that is multiple of 4. For non constant offsets we will always use<br>
byte_scattered_write.<br></blockquote><div><br></div><div>I double-checked the rules and we can't even guarantee that a f16vec2 is dword-aligned.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
---<br>
 src/intel/compiler/brw_fs_nir.<wbr>cpp | 22 +++++++++++++++-------<br>
 1 file changed, 15 insertions(+), 7 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 45b8e8b637..abf9098252 100644<br>
--- a/src/intel/compiler/brw_fs_<wbr>nir.cpp<br>
+++ b/src/intel/compiler/brw_fs_<wbr>nir.cpp<br>
@@ -4135,6 +4135,8 @@ fs_visitor::nir_emit_<wbr>intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr<br>
          unsigned num_components = ffs(~(writemask >> first_component)) - 1;<br>
          fs_reg write_src = offset(val_reg, bld, first_component);<br>
<br>
+         nir_const_value *const_offset = nir_src_as_const_value(instr-><wbr>src[2]);<br>
+<br>
          if (type_size > 4) {<br>
             /* We can't write more than 2 64-bit components at once. Limit<br>
              * the num_components of the write to what we can do and let the next<br>
@@ -4150,14 +4152,19 @@ fs_visitor::nir_emit_<wbr>intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr<br>
              * 32-bit-aligned we need to use byte-scattered writes because<br>
              * untyped writes works with 32-bit components with 32-bit<br>
              * alignment. byte_scattered_write messages only support one<br>
-             * 16-bit component at a time.<br>
+             * 16-bit component at a time. As VK_KHR_relaxed_block_layout<br>
+             * could be enabled we can not guarantee that not constant offsets<br>
+             * to be 32-bit aligned for 16-bit types. For example an array, of<br>
+             * 16-bit vec3 with array element stride of 6.<br>
              *<br>
-             * For example, if there is a 3-components vector we submit one<br>
-             * untyped-write message of 32-bit (first two components), and one<br>
-             * byte-scattered write message (the last component).<br>
+             * In the case of 32-bit aligned constant offsets if there is<br>
+             * a 3-components vector we submit one untyped-write message<br>
+             * of 32-bit (first two components), and one byte-scattered<br>
+             * write message (the last component).<br>
              */<br>
<br>
-            if (first_component % 2) {<br>
+            if ( !const_offset || ((const_offset->u32[0] +<br>
+                                   type_size * first_component) % 4)) {<br>
                /* If we use a .yz writemask we also need to emit 2<br>
                 * byte-scattered write messages because of y-component not<br>
                 * being aligned to 32-bit.<br>
@@ -4183,7 +4190,7 @@ fs_visitor::nir_emit_<wbr>intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr<br>
          }<br>
<br>
          fs_reg offset_reg;<br>
-         nir_const_value *const_offset = nir_src_as_const_value(instr-><wbr>src[2]);<br>
+<br>
          if (const_offset) {<br>
             offset_reg = brw_imm_ud(const_offset->u32[<wbr>0] +<br>
                                     type_size * first_component);<br>
@@ -4222,7 +4229,8 @@ fs_visitor::nir_emit_<wbr>intrinsic(const fs_builder &bld, nir_intrinsic_instr *instr<br>
          } else {<br>
             assert(num_components * type_size <= 16);<br>
             assert((num_components * type_size) % 4 == 0);<br>
-            assert((first_component * type_size) % 4 == 0);<br>
+            assert(!const_offset ||<br>
+                   (const_offset->u32[0] + type_size * first_component) % 4 == 0);<br></blockquote><div><br></div><div>How about<br><br></div><div>assert(offset_reg.file != BRW_IMMEDIATE_VALUE || offset_reg.ud % 4 == 0);<br><br></div><div>We've already done the above calculation and stored it in offset_reg.<br><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
             unsigned num_slots = (num_components * type_size) / 4;<br>
<br>
             emit_untyped_write(bld, surf_index, offset_reg,<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.14.3<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>