On Jul 27, 2016 6:03 AM, "Emil Velikov" <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>> wrote: > > From: Emil Velikov <<a href="mailto:emil.velikov@collabora.com">emil.velikov@collabora.com</a>> > > Presently the layer has only a single entry point, which uses asserts solely. > Thus even on release builds the function (and thus the whole layer) will do > nothing but adding runtime and binary(size) overhead. > > Cc: "12.0" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>> > Cc: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > Signed-off-by: Emil Velikov <<a href="mailto:emil.velikov@collabora.com">emil.velikov@collabora.com</a>> > --- > We can use a ANV_USE_VALIDATE macro which atm will default to == DEBUG. > --- > src/intel/vulkan/anv_entrypoints_gen.py | 36 +++++++++++++++++++++++---------- > src/intel/vulkan/anv_image.c | 2 ++ > 2 files changed, 27 insertions(+), 11 deletions(-) > > diff --git a/src/intel/vulkan/anv_entrypoints_gen.py b/src/intel/vulkan/anv_entrypoints_gen.py > index 2896174..a4922c2 100644 > --- a/src/intel/vulkan/anv_entrypoints_gen.py > +++ b/src/intel/vulkan/anv_entrypoints_gen.py > @@ -134,7 +134,9 @@ if opt_header: > print "%s gen75_%s%s;" % (type, name, args) > print "%s gen8_%s%s;" % (type, name, args) > print "%s gen9_%s%s;" % (type, name, args) > + print "#ifdef DEBUG" > print "%s anv_validate_%s%s;" % (type, name, args) > + print "#endif // DEBUG" > print_guard_end(name) > exit() > > @@ -185,14 +187,7 @@ for type, name, args, num, h in entrypoints: > print " \"vk%s\\0\"" % name > offsets.append(i) > i += 2 + len(name) + 1 > -print """ ; > - > -/* Weak aliases for all potential validate functions. These will resolve to > - * NULL if they're not defined, which lets the resolve_entrypoint() function > - * either pick a validate wrapper if available or just plug in the actual > - * entry point. > - */ > -""" > +print " ;" > > # Now generate the table of all entry points and their validation functions > > @@ -201,7 +196,18 @@ for type, name, args, num, h in entrypoints: > print " { %5d, 0x%08x }," % (offsets[num], h) > print "};\n" > > +print """ > + > +/* Weak aliases for all potential validate functions. These will resolve to > + * NULL if they're not defined, which lets the resolve_entrypoint() function > + * either pick a validate wrapper if available or just plug in the actual > + * entry point. > + */ > +""" > + > for layer in [ "anv", "validate", "gen7", "gen75", "gen8", "gen9" ]: > + if "validate" in layer: > + print "#ifdef DEBUG" > for type, name, args, num, h in entrypoints: > print_guard_start(name) > print "%s %s_%s%s __attribute__ ((weak));" % (type, layer, name, args) > @@ -211,13 +217,13 @@ for layer in [ "anv", "validate", "gen7", "gen75", "gen8", "gen9" ]: > print_guard_start(name) > print " .%s = %s_%s," % (name, layer, name) > print_guard_end(name) > + if "validate" in layer: > + print "#endif // DEBUG" > print "};\n" > > print """ > #ifdef DEBUG > static bool enable_validate = true; > -#else > -static bool enable_validate = false; > #endif > > /* We can't use symbols that need resolving (like, oh, getenv) in the resolve > @@ -231,8 +237,14 @@ determine_validate(void) > { > const char *s = getenv("ANV_VALIDATE"); > > - if (s) > + if (s) { > +#ifdef DEBUG > enable_validate = atoi(s); > +#else > + fprintf(stderr, "libvulkand-intel was built without VALIDATE support.\\n"); > + abort(); > +#endif > + } > } > > static const struct brw_device_info *dispatch_devinfo; > @@ -246,8 +258,10 @@ anv_set_dispatch_devinfo(const struct brw_device_info *devinfo) > void * __attribute__ ((noinline)) > anv_resolve_entrypoint(uint32_t index) > { > +#ifdef DEBUG > if (enable_validate && validate_layer.entrypoints[index]) > return validate_layer.entrypoints[index]; > +#endif > > if (dispatch_devinfo == NULL) { > return anv_layer.entrypoints[index]; > diff --git a/src/intel/vulkan/anv_image.c b/src/intel/vulkan/anv_image.c > index dff51bc..43d19f5 100644 > --- a/src/intel/vulkan/anv_image.c > +++ b/src/intel/vulkan/anv_image.c > @@ -332,6 +332,7 @@ void anv_GetImageSubresourceLayout( > } > } > > +#ifdef DEBUG > VkResult > anv_validate_CreateImageView(VkDevice _device, As long as anv_entrypoints contains a declaration of this function, we don't need to #ifdef around it. Hopefully, the linker is smart enough to detect that it's not used and delete the code. Otherwise, this patch seems perfectly reasonable for now. At one point we had the idea that we could use the validate endpoints to do some extra validation that would be shipped in release mode but disabled by default at zero cost. However, we're not really taking advantage of that now and do we might as well drop it. > const VkImageViewCreateInfo *pCreateInfo, > @@ -406,6 +407,7 @@ anv_validate_CreateImageView(VkDevice _device, > > return anv_CreateImageView(_device, pCreateInfo, pAllocator, pView); > } > +#endif // DEBUG > > static struct anv_state > alloc_surface_state(struct anv_device *device, > -- > 2.9.0 >