<html><head></head><body><div>On Thu, 2017-04-27 at 20:36 -0700, Jason Ekstrand wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Apr 27, 2017 at 9:23 AM, Juan A. Suarez Romero <span dir="ltr"><<a href="mailto:jasuarez@igalia.com" target="_blank">jasuarez@igalia.com</a>></span> wrote:<br><blockquote type="cite"><div class="HOEnZb"><div class="h5">On Wed, 2017-04-26 at 07:35 -0700, Jason Ekstrand wrote:<br>
> We should only use size_t when referring to sizes of bits of CPU memory.<br>
> Anything on the GPU or just a regular array length should be a type that<br>
> has the same size on both 32 and 64-bit architectures.  For state<br>
> objects, we use a uint32_t because we'll never allocate a piece of<br>
> driver-internal GPU state larger than 2GB (more like 16KB).<br>
> ---<br>
>  src/intel/vulkan/anv_<wbr>allocator.c | 12 ++++++------<br>
>  src/intel/vulkan/anv_gem.c       |  2 +-<br>
>  src/intel/vulkan/anv_gem_<wbr>stubs.c |  2 +-<br>
>  src/intel/vulkan/anv_private.h   | 12 ++++++------<br>
>  4 files changed, 14 insertions(+), 14 deletions(-)<br>
><br>
> diff --git a/src/intel/vulkan/anv_<wbr>allocator.c b/src/intel/vulkan/anv_<wbr>allocator.c<br>
> index 1c94d1b..2dad400 100644<br>
> --- a/src/intel/vulkan/anv_<wbr>allocator.c<br>
> +++ b/src/intel/vulkan/anv_<wbr>allocator.c<br>
> @@ -342,7 +342,7 @@ anv_block_pool_finish(struct anv_block_pool *pool)<br>
>  static uint32_t<br>
>  anv_block_pool_grow(struct anv_block_pool *pool, struct anv_block_state *state)<br>
>  {<br>
> -   size_t size;<br>
> +   uint32_t size;<br>
>     void *map;<br>
>     uint32_t gem_handle;<br>
>     struct anv_mmap_cleanup *cleanup;<br>
> @@ -367,7 +367,7 @@ anv_block_pool_grow(struct anv_block_pool *pool, struct anv_block_state *state)<br>
><br>
>     assert(state == &pool->state || back_used > 0);<br>
><br>
> -   size_t old_size = pool->bo.size;<br>
> +   uint32_t old_size = pool->bo.size;<br>
><br>
>     if (old_size != 0 &&<br>
>         back_used * 2 <= pool->center_bo_offset &&<br>
<br>
<br>
</div></div>While checking this patch, I've notice we have the following comment in<br>
the anv_block_pool_grow()<br>
<br>
   /* We can't have a block pool bigger than 1GB because we use signed<br>
    * 32-bit offsets in the free list and we don't want overflow.  We<br>
    * should never need a block pool bigger than 1GB anyway.<br>
    */<br>
   assert(size <= (1u << 31));<br>
<br>
Is the comment correct? Shouldn't say 2Gb?<br></blockquote><div><br></div><div>1 or 2; I can't remember at the moment.  I thought there was something weird with Android that required us to drop it to 1.<br></div><div> </div></div></div></div></blockquote><div><br></div><div>If it's 1Gb, I think assertion should be "assert(size <= (1u << 30));"</div><div><br></div><div><br></div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote type="cite">
Other than that,<br>
<br>
Reviewed-by: Juan A. Suarez Romero <<a href="mailto:jasuarez@igalia.com">jasuarez@igalia.com</a>><br>
<div><div class="h5"><br>
<br>
> @@ -613,7 +613,7 @@ anv_block_pool_free(struct anv_block_pool *pool, int32_t offset)<br>
><br>
>  static void<br>
>  anv_fixed_size_state_pool_<wbr>init(struct anv_fixed_size_state_pool *pool,<br>
> -                               size_t state_size)<br>
> +                               uint32_t state_size)<br>
>  {<br>
>     /* At least a cache line and must divide the block size. */<br>
>     assert(state_size >= 64 && util_is_power_of_two(state_<wbr>size));<br>
> @@ -672,7 +672,7 @@ anv_state_pool_init(struct anv_state_pool *pool,<br>
>  {<br>
>     pool->block_pool = block_pool;<br>
>     for (unsigned i = 0; i < ANV_STATE_BUCKETS; i++) {<br>
> -      size_t size = 1 << (ANV_MIN_STATE_SIZE_LOG2 + i);<br>
> +      uint32_t size = 1 << (ANV_MIN_STATE_SIZE_LOG2 + i);<br>
>        anv_fixed_size_state_pool_<wbr>init(&pool->buckets[i], size);<br>
>     }<br>
>     VG(VALGRIND_CREATE_MEMPOOL(<wbr>pool, 0, false));<br>
> @@ -686,7 +686,7 @@ anv_state_pool_finish(struct anv_state_pool *pool)<br>
><br>
>  static struct anv_state<br>
>  anv_state_pool_alloc_no_vg(<wbr>struct anv_state_pool *pool,<br>
> -                           size_t size, size_t align)<br>
> +                           uint32_t size, uint32_t align)<br>
>  {<br>
>     unsigned size_log2 = ilog2_round_up(size < align ? align : size);<br>
>     assert(size_log2 <= ANV_MAX_STATE_SIZE_LOG2);<br>
> @@ -703,7 +703,7 @@ anv_state_pool_alloc_no_vg(<wbr>struct anv_state_pool *pool,<br>
>  }<br>
><br>
>  struct anv_state<br>
> -anv_state_pool_alloc(struct anv_state_pool *pool, size_t size, size_t align)<br>
> +anv_state_pool_alloc(struct anv_state_pool *pool, uint32_t size, uint32_t align)<br>
>  {<br>
>     if (size == 0)<br>
>        return ANV_STATE_NULL;<br>
> diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c<br>
> index 185086f..4b6ee58 100644<br>
> --- a/src/intel/vulkan/anv_gem.c<br>
> +++ b/src/intel/vulkan/anv_gem.c<br>
> @@ -48,7 +48,7 @@ anv_ioctl(int fd, unsigned long request, void *arg)<br>
>   * Return gem handle, or 0 on failure. Gem handles are never 0.<br>
>   */<br>
>  uint32_t<br>
> -anv_gem_create(struct anv_device *device, size_t size)<br>
> +anv_gem_create(struct anv_device *device, uint64_t size)<br>
>  {<br>
>     struct drm_i915_gem_create gem_create = {<br>
>        .size = size,<br>
> diff --git a/src/intel/vulkan/anv_gem_<wbr>stubs.c b/src/intel/vulkan/anv_gem_<wbr>stubs.c<br>
> index a63e96d..8d81eb5 100644<br>
> --- a/src/intel/vulkan/anv_gem_<wbr>stubs.c<br>
> +++ b/src/intel/vulkan/anv_gem_<wbr>stubs.c<br>
> @@ -34,7 +34,7 @@ memfd_create(const char *name, unsigned int flags)<br>
>  }<br>
><br>
>  uint32_t<br>
> -anv_gem_create(struct anv_device *device, size_t size)<br>
> +anv_gem_create(struct anv_device *device, uint64_t size)<br>
>  {<br>
>     int fd = memfd_create("fake bo", MFD_CLOEXEC);<br>
>     if (fd == -1)<br>
> diff --git a/src/intel/vulkan/anv_<wbr>private.h b/src/intel/vulkan/anv_<wbr>private.h<br>
> index 216c6eb..c709d39 100644<br>
> --- a/src/intel/vulkan/anv_<wbr>private.h<br>
> +++ b/src/intel/vulkan/anv_<wbr>private.h<br>
> @@ -493,7 +493,7 @@ struct anv_state {<br>
>  #define ANV_STATE_NULL ((struct anv_state) { .alloc_size = 0 })<br>
><br>
>  struct anv_fixed_size_state_pool {<br>
> -   size_t state_size;<br>
> +   uint32_t state_size;<br>
>     union anv_free_list free_list;<br>
>     struct anv_block_state block;<br>
>  };<br>
> @@ -565,7 +565,7 @@ void anv_state_pool_init(struct anv_state_pool *pool,<br>
>                           struct anv_block_pool *block_pool);<br>
>  void anv_state_pool_finish(struct anv_state_pool *pool);<br>
>  struct anv_state anv_state_pool_alloc(struct anv_state_pool *pool,<br>
> -                                      size_t state_size, size_t alignment);<br>
> +                                      uint32_t state_size, uint32_t alignment);<br>
>  void anv_state_pool_free(struct anv_state_pool *pool, struct anv_state state);<br>
>  void anv_state_stream_init(struct anv_state_stream *stream,<br>
>                             struct anv_state_pool *state_pool,<br>
> @@ -752,7 +752,7 @@ VkResult anv_device_wait(struct anv_device *device, struct anv_bo *bo,<br>
>  void* anv_gem_mmap(struct anv_device *device,<br>
>                     uint32_t gem_handle, uint64_t offset, uint64_t size, uint32_t flags);<br>
>  void anv_gem_munmap(void *p, uint64_t size);<br>
> -uint32_t anv_gem_create(struct anv_device *device, size_t size);<br>
> +uint32_t anv_gem_create(struct anv_device *device, uint64_t size);<br>
>  void anv_gem_close(struct anv_device *device, uint32_t gem_handle);<br>
>  uint32_t anv_gem_userptr(struct anv_device *device, void *mem, size_t size);<br>
>  int anv_gem_busy(struct anv_device *device, uint32_t gem_handle);<br>
> @@ -780,8 +780,8 @@ int anv_gem_set_domain(struct anv_device *device, uint32_t gem_handle,<br>
>  VkResult anv_bo_init_new(struct anv_bo *bo, struct anv_device *device, uint64_t size);<br>
><br>
>  struct anv_reloc_list {<br>
> -   size_t                                       num_relocs;<br>
> -   size_t                                       array_length;<br>
> +   uint32_t                                     num_relocs;<br>
> +   uint32_t                                     array_length;<br>
>     struct drm_i915_gem_relocation_entry *       relocs;<br>
>     struct anv_bo **                             reloc_bos;<br>
>  };<br>
> @@ -803,7 +803,7 @@ struct anv_batch_bo {<br>
>     struct anv_bo                                bo;<br>
><br>
>     /* Bytes actually consumed in this batch BO */<br>
> -   size_t                                       length;<br>
> +   uint32_t                                     length;<br>
><br>
>     struct anv_reloc_list                        relocs;<br>
>  };<br>
</div></div>______________________________<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>
<br></blockquote></div><br></div></div>
</blockquote></body></html>