nir: Scalarize bounds checked loads and stores

Fixes: 39da1deb497a ("nir/lower_io: Add a bounds-checked 64-bit global address format")
Reviewed-by: M Henning <drawoc@darkrefraction.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26526>
(cherry picked from commit 1cf1b9d7412e94f70a5f68f81eed7ac22ad75613)

diff --git a/.pick_status.json b/.pick_status.json
index 384d551..07a2d0f 100644 (file)
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -604,7 +604,7 @@
         "description": "nir: Scalarize bounds checked loads and stores",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": "39da1deb497af55496308c0867b5ce5a0e9df56e",
         "notes": null

diff --git a/src/compiler/nir/nir_lower_io.c b/src/compiler/nir/nir_lower_io.c
index d2bca8a..58d6966 100644 (file)
--- a/src/compiler/nir/nir_lower_io.c
+++ b/src/compiler/nir/nir_lower_io.c
@@ -1570,7 +1570,8 @@ build_explicit_io_load(nir_builder *b, nir_intrinsic_instr *intrin,
       nir_def *zero = nir_imm_zero(b, load->num_components, bit_size);
 
       /* TODO: Better handle block_intel. */
-      const unsigned load_size = (bit_size / 8) * load->num_components;
+      assert(load->num_components == 1);
+      const unsigned load_size = bit_size / 8;
       nir_push_if(b, addr_is_in_bounds(b, addr, addr_format, load_size));
 
       nir_builder_instr_insert(b, &load->instr);
@@ -1755,7 +1756,8 @@ build_explicit_io_store(nir_builder *b, nir_intrinsic_instr *intrin,
 
    if (addr_format_needs_bounds_check(addr_format)) {
       /* TODO: Better handle block_intel. */
-      const unsigned store_size = (value->bit_size / 8) * store->num_components;
+      assert(store->num_components == 1);
+      const unsigned store_size = value->bit_size / 8;
       nir_push_if(b, addr_is_in_bounds(b, addr, addr_format, store_size));
 
       nir_builder_instr_insert(b, &store->instr);
@@ -1948,8 +1950,12 @@ nir_lower_explicit_io_instr(nir_builder *b,
    nir_deref_instr *deref = nir_src_as_deref(intrin->src[0]);
    unsigned vec_stride = glsl_get_explicit_stride(deref->type);
    unsigned scalar_size = type_scalar_size_bytes(deref->type);
-   assert(vec_stride == 0 || glsl_type_is_vector(deref->type));
-   assert(vec_stride == 0 || vec_stride >= scalar_size);
+   if (vec_stride == 0) {
+      vec_stride = scalar_size;
+   } else {
+      assert(glsl_type_is_vector(deref->type));
+      assert(vec_stride >= scalar_size);
+   }
 
    uint32_t align_mul, align_offset;
    if (!nir_get_explicit_deref_align(deref, true, &align_mul, &align_offset)) {
@@ -1958,10 +1964,27 @@ nir_lower_explicit_io_instr(nir_builder *b,
       align_offset = 0;
    }
 
+   /* In order for bounds checking to be correct as per the Vulkan spec,
+    * we need to check at the individual component granularity.  Prior to
+    * robustness2, we're technically allowed to be sloppy by 16B.  Even with
+    * robustness2, UBO loads are allowed to have a granularity as high as 256B
+    * depending on hardware limits.  However, we have none of that information
+    * here.  Short of adding new address formats, the easiest way to do that
+    * is to just split any loads and stores into individual components here.
+    *
+    * TODO: At some point in the future we may want to add more ops similar to
+    * nir_intrinsic_load_global_constant_bounded and make bouds checking the
+    * back-end's problem.  Another option would be to somehow plumb more of
+    * that information through to nir_lower_explicit_io.  For now, however,
+    * scalarizing is at least correct.
+    */
+   bool scalarize = vec_stride > scalar_size ||
+                    addr_format_needs_bounds_check(addr_format);
+
    switch (intrin->intrinsic) {
    case nir_intrinsic_load_deref: {
       nir_def *value;
-      if (vec_stride > scalar_size) {
+      if (scalarize) {
          nir_def *comps[NIR_MAX_VEC_COMPONENTS] = {
             NULL,
          };
@@ -1990,7 +2013,7 @@ nir_lower_explicit_io_instr(nir_builder *b,
    case nir_intrinsic_store_deref: {
       nir_def *value = intrin->src[1].ssa;
       nir_component_mask_t write_mask = nir_intrinsic_write_mask(intrin);
-      if (vec_stride > scalar_size) {
+      if (scalarize) {
          for (unsigned i = 0; i < intrin->num_components; i++) {
             if (!(write_mask & (1 << i)))
                continue;