On 2018-05-09 08:20:59, Scott D Phillips wrote:
> Jason Ekstrand <ja...@jlekstrand.net> writes:
>
> > On Tue, May 8, 2018 at 10:58 AM, Jordan Justen <jordan.l.jus...@intel.com>
> > wrote:
> >
> >> On 2018-05-07 17:30:43, Scott D Phillips wrote:
> >> > From: Jason Ekstrand <jason.ekstr...@intel.com>
> >> >
> >> > This is simple linear-walk first-fit allocator roughly based on the
> >> > allocator in the radeon winsys code. This allocator has two primary
> >> > functional differences:
> >> >
> >> > 1) It cleanly returns 0 on allocation failure
> >> >
> >> > 2) It allocates addresses top-down instead of bottom-up.
> >> >
> >> > The second one is needed for Intel because high addresses (with bit 47
> >> > set) need to be canonicalized in order to work properly. If we allocate
> >> > bottom-up, then high addresses will be very rare (if they ever happen).
> >> > We'd rather always have high addresses so that the canonicalization code
> >> > gets better testing.
> >> >
> >> > Reviewed-by: Scott D Phillips <scott.d.phill...@intel.com>
> >> > Tested-by: Scott D Phillips <scott.d.phill...@intel.com>
> >> > ---
> >> > src/util/Makefile.sources | 4 +-
> >> > src/util/meson.build | 2 +
> >> > src/util/vma.c | 231 ++++++++++++++++++++++++++++++
> >> ++++++++++++++++
> >> > src/util/vma.h | 53 +++++++++++
> >> > 4 files changed, 289 insertions(+), 1 deletion(-)
> >> > create mode 100644 src/util/vma.c
> >> > create mode 100644 src/util/vma.h
> >> >
> >
> > [snip]
> >
> >> > +uint64_t
> >> > +util_vma_heap_alloc(struct util_vma_heap *heap,
> >> > + uint64_t size, uint64_t alignment)
> >> > +{
> >> > + /* The caller is expected to reject zero-size allocations */
> >> > + assert(size > 0);
> >> > +
> >> > + assert(util_is_power_of_two_nonzero(alignment));
> >> > +
> >> > + util_vma_heap_validate(heap);
> >>
> >> It looks like the loop in util_vma_heap_validate contains only
> >> asserts, thus it seems possible that on a release build the compiler
> >> might eliminate the list walk entirely.
> >>
> >> But, we could also make util_vma_heap_validate return a bool, and make
> >> all calls assert(util_vma_heap_validate(heap)).
> >>
> >
> > That's probably a good idea. If we do that, we'll need to make
> > util_vma_heap_validate inline so we don't get warnings on release builds.
> > Another option would be to surround the contents of the function with
> > "#ifndef NDEBUG"
>
> I just checked the built machine code from release builds made with gcc
> 7.3.1 and clang 6.0.0, they both completely remove
> util_vma_heap_validate. Would you still like to see the change suggested
> here?
Cool. Thanks for checking it. The ifndef option seems to be an easy
rework, and (I think) still worthwhile.
-Jordan
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
