anv: Push UBO ranges relative to the start of the binding

There was a disconnect between anv_nir_compute_push_layout and the code
which sets up the push_ubo_sizes array.  The NIR code we emit checks
relative to the start of the bound UBO range so that, if we end up with
a vector which straddles the start of the push range, we can perform the
bounds check without risking overflow issues.  The code which sets up
the push_ubo_sizes, on the other hand, assumed it was relative to the
start of the push range.  Somehow, this didn't get get caught by any of
the available tests.

Fixes: e03f9652801 "anv: Bounds-check pushed UBOs when ..."
Closes: #2623
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4195>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4195>

diff --git a/src/intel/vulkan/anv_nir_compute_push_layout.c b/src/intel/vulkan/anv_nir_compute_push_layout.c
index f13b9e1aa38700dcdaf37e8bcc779c939643986f..3f9572644df18d8522f1e8591a1ccf9973b7f728 100644 (file)
--- a/src/intel/vulkan/anv_nir_compute_push_layout.c
+++ b/src/intel/vulkan/anv_nir_compute_push_layout.c
@@ -250,6 +250,13 @@ anv_nir_compute_push_layout(const struct anv_physical_device *pdevice,
                    * layout, we could end up with a single vector straddling a
                    * 32B boundary.
                    *
+                   * We intentionally push a size starting from the UBO
+                   * binding in the descriptor set rather than starting from
+                   * the started of the pushed range.  This prevents us from
+                   * accidentally flagging things as out-of-bounds due to
+                   * roll-over if a vector access crosses the push range
+                   * boundary.
+                   *
                    * We align up to 32B so that we can get better CSE.
                    *
                    * We check

diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c
index 49a9e8c6913c894e5801829f6c90e465d2102691..a37bc96b216cf16d5ff08a856536f0d474ab2e4a 100644 (file)
--- a/src/intel/vulkan/genX_cmd_buffer.c
+++ b/src/intel/vulkan/genX_cmd_buffer.c
@@ -2924,9 +2924,12 @@ get_push_range_address(struct anv_cmd_buffer *cmd_buffer,
 }
 
 
-/** Returns the size in bytes of the bound buffer relative to range->start
+/** Returns the size in bytes of the bound buffer
  *
- * This may be smaller than range->length * 32.
+ * The range is relative to the start of the buffer, not the start of the
+ * range.  The returned range may be smaller than
+ *
+ *    (range->start + range->length) * 32;
  */
 static uint32_t
 get_push_range_bound_size(struct anv_cmd_buffer *cmd_buffer,
@@ -2941,11 +2944,11 @@ get_push_range_bound_size(struct anv_cmd_buffer *cmd_buffer,
          gfx_state->base.descriptors[range->index];
       assert(range->start * 32 < set->desc_mem.alloc_size);
       assert((range->start + range->length) * 32 <= set->desc_mem.alloc_size);
-      return set->desc_mem.alloc_size - range->start * 32;
+      return set->desc_mem.alloc_size;
    }
 
    case ANV_DESCRIPTOR_SET_PUSH_CONSTANTS:
-      return range->length * 32;
+      return (range->start + range->length) * 32;
 
    default: {
       assert(range->set < MAX_SETS);
@@ -2955,10 +2958,7 @@ get_push_range_bound_size(struct anv_cmd_buffer *cmd_buffer,
          &set->descriptors[range->index];
 
       if (desc->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER) {
-         if (range->start * 32 > desc->buffer_view->range)
-            return 0;
-
-         return desc->buffer_view->range - range->start * 32;
+         return desc->buffer_view->range;
       } else {
          assert(desc->type == VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC);
          /* Compute the offset within the buffer */
@@ -2975,10 +2975,7 @@ get_push_range_bound_size(struct anv_cmd_buffer *cmd_buffer,
          /* Align the range for consistency */
          bound_range = align_u32(bound_range, ANV_UBO_BOUNDS_CHECK_ALIGNMENT);
 
-         if (range->start * 32 > bound_range)
-            return 0;
-
-         return bound_range - range->start * 32;
+         return bound_range;
       }
    }
    }