Re: [Mesa-dev] [PATCH v2 01/11] vulkan: util: add macros to extract extension/offset number from enums

2017-10-04 Thread Lionel Landwerlin 

On 04/10/17 00:40, Chad Versace wrote:

On Tue 03 Oct 2017, Jason Ekstrand wrote:

On Tue, Oct 3, 2017 at 3:18 PM, Lionel Landwerlin <[1]
lionel.g.landwer...@intel.com> wrote:

 On 03/10/17 21:21, Chad Versace wrote:

 On Tue 03 Oct 2017, Lionel Landwerlin wrote:

 On 03/10/17 19:13, Jason Ekstrand wrote:

      +1 to static inline

 Done locally.

 Cool. Waiting to see it appear in wip/djeath/ycbcr_conversion.


 Ah...
 I didn't actually test that (with all the other commits on top).

 Unfortunately that's breaking a bit the way we index formats :

 [4]https://github.com/djdeath/mesa/blob/wip/djdeath/ycbcr_conversion/src/
 intel/vulkan/anv_formats.c#L49


Right... That's a bummer.  Macros it is, I guess.

Fair enough. But please make the macros uppercase, so no one is hurt by
the multiple evaluation.

I updated the branch on github. The first 7 patches should have 
addressed the review comments.

Still need to deal with array of samplers in the nir pass.

I can resend if you want.

-
Lionel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 01/11] vulkan: util: add macros to extract extension/offset number from enums

2017-10-03 Thread Chad Versace 
On Tue 03 Oct 2017, Jason Ekstrand wrote:
> 
> On Tue, Oct 3, 2017 at 3:18 PM, Lionel Landwerlin <[1]
> lionel.g.landwer...@intel.com> wrote:
> 
> On 03/10/17 21:21, Chad Versace wrote:
> 
> On Tue 03 Oct 2017, Lionel Landwerlin wrote:
> 
> On 03/10/17 19:13, Jason Ekstrand wrote:
> 
>      +1 to static inline
> 
> Done locally.
> 
> Cool. Waiting to see it appear in wip/djeath/ycbcr_conversion.
> 
> 
> Ah...
> I didn't actually test that (with all the other commits on top).
> 
> Unfortunately that's breaking a bit the way we index formats :
> 
> [4]https://github.com/djdeath/mesa/blob/wip/djdeath/ycbcr_conversion/src/
> intel/vulkan/anv_formats.c#L49
> 
> 
> Right... That's a bummer.  Macros it is, I guess.

Fair enough. But please make the macros uppercase, so no one is hurt by
the multiple evaluation.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 01/11] vulkan: util: add macros to extract extension/offset number from enums

2017-10-03 Thread Jason Ekstrand 
On Tue, Oct 3, 2017 at 3:18 PM, Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:

> On 03/10/17 21:21, Chad Versace wrote:
>
>> On Tue 03 Oct 2017, Lionel Landwerlin wrote:
>>
>>> On 03/10/17 19:13, Jason Ekstrand wrote:
>>>
>>>  On Tue, Oct 3, 2017 at 9:43 AM, Chad Versace <[1]
>>> chadvers...@chromium.org>
>>>  wrote:
>>>
>>>  On Tue 03 Oct 2017, Lionel Landwerlin wrote:
>>>  > v2: Simplify offset enum computation (Jason)
>>>  >
>>>  > Signed-off-by: Lionel Landwerlin <[2]
>>> lionel.g.landwer...@intel.com>
>>>  > ---
>>>  >  src/vulkan/util/vk_util.h | 6 ++
>>>  >  1 file changed, 6 insertions(+)
>>>  >
>>>  > diff --git a/src/vulkan/util/vk_util.h
>>> b/src/vulkan/util/vk_util.h
>>>  > index 2ed601f881e..8c8cb64d513 100644
>>>  > --- a/src/vulkan/util/vk_util.h
>>>  > +++ b/src/vulkan/util/vk_util.h
>>>  > @@ -199,4 +199,10 @@ __vk_find_struct(void *start,
>>> VkStructureType
>>>  sType)
>>>  >
>>>  >  uint32_t vk_get_driver_version(void);
>>>  >
>>>  > +#define VK_EXT_OFFSET (10UL)
>>>  > +#define vk_enum_extension(__enum) \
>>>  > +   ((__enum) >= VK_EXT_OFFSET ? __enum) - VK_EXT_OFFSET)
>>> /
>>>  1000UL) + 1) : 0)
>>>  > +#define vk_enum_offset(__enum) \
>>>  > +   ((__enum) >= VK_EXT_OFFSET ? ((__enum) % 1000) : (__enum))
>>>
>>>  The macro functions, when called, look like regular functions
>>> due to
>>>  being lowercase. But they don't behave like functions; their
>>> arguments
>>>  suffer from the multiple evaluation disease.
>>>
>>>  Please rename the macros to be all uppercase, so callers'
>>> expectations
>>>  will be set correctly. Or, even better, define them as inline
>>>  functions.
>>>
>>>
>>>  +1 to static inline
>>>
>>> Done locally.
>>>
>> Cool. Waiting to see it appear in wip/djeath/ycbcr_conversion.
>>
>
> Ah...
> I didn't actually test that (with all the other commits on top).
>
> Unfortunately that's breaking a bit the way we index formats :
>
> https://github.com/djdeath/mesa/blob/wip/djdeath/ycbcr_conve
> rsion/src/intel/vulkan/anv_formats.c#L49


Right... That's a bummer.  Macros it is, I guess.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 01/11] vulkan: util: add macros to extract extension/offset number from enums

2017-10-03 Thread Lionel Landwerlin 

On 03/10/17 21:21, Chad Versace wrote:

On Tue 03 Oct 2017, Lionel Landwerlin wrote:

On 03/10/17 19:13, Jason Ekstrand wrote:

 On Tue, Oct 3, 2017 at 9:43 AM, Chad Versace <[1]chadvers...@chromium.org>
 wrote:

 On Tue 03 Oct 2017, Lionel Landwerlin wrote:
 > v2: Simplify offset enum computation (Jason)
 >
 > Signed-off-by: Lionel Landwerlin <[2]lionel.g.landwer...@intel.com>
 > ---
 >  src/vulkan/util/vk_util.h | 6 ++
 >  1 file changed, 6 insertions(+)
 >
 > diff --git a/src/vulkan/util/vk_util.h b/src/vulkan/util/vk_util.h
 > index 2ed601f881e..8c8cb64d513 100644
 > --- a/src/vulkan/util/vk_util.h
 > +++ b/src/vulkan/util/vk_util.h
 > @@ -199,4 +199,10 @@ __vk_find_struct(void *start, VkStructureType
 sType)
 >
 >  uint32_t vk_get_driver_version(void);
 >
 > +#define VK_EXT_OFFSET (10UL)
 > +#define vk_enum_extension(__enum) \
 > +   ((__enum) >= VK_EXT_OFFSET ? __enum) - VK_EXT_OFFSET) /
 1000UL) + 1) : 0)
 > +#define vk_enum_offset(__enum) \
 > +   ((__enum) >= VK_EXT_OFFSET ? ((__enum) % 1000) : (__enum))

 The macro functions, when called, look like regular functions due to
 being lowercase. But they don't behave like functions; their arguments
 suffer from the multiple evaluation disease.

 Please rename the macros to be all uppercase, so callers' expectations
 will be set correctly. Or, even better, define them as inline
 functions.


 +1 to static inline

Done locally.

Cool. Waiting to see it appear in wip/djeath/ycbcr_conversion.


Ah...
I didn't actually test that (with all the other commits on top).

Unfortunately that's breaking a bit the way we index formats :

https://github.com/djdeath/mesa/blob/wip/djdeath/ycbcr_conversion/src/intel/vulkan/anv_formats.c#L49



References:

[1] mailto:chadvers...@chromium.org
[2] mailto:lionel.g.landwer...@intel.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 01/11] vulkan: util: add macros to extract extension/offset number from enums

2017-10-03 Thread Chad Versace 
On Tue 03 Oct 2017, Lionel Landwerlin wrote:
> On 03/10/17 19:13, Jason Ekstrand wrote:
> 
> On Tue, Oct 3, 2017 at 9:43 AM, Chad Versace <[1]chadvers...@chromium.org>
> wrote:
> 
> On Tue 03 Oct 2017, Lionel Landwerlin wrote:
> > v2: Simplify offset enum computation (Jason)
> >
> > Signed-off-by: Lionel Landwerlin <[2]lionel.g.landwer...@intel.com>
> > ---
> >  src/vulkan/util/vk_util.h | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/vulkan/util/vk_util.h b/src/vulkan/util/vk_util.h
> > index 2ed601f881e..8c8cb64d513 100644
> > --- a/src/vulkan/util/vk_util.h
> > +++ b/src/vulkan/util/vk_util.h
> > @@ -199,4 +199,10 @@ __vk_find_struct(void *start, VkStructureType
> sType)
> >
> >  uint32_t vk_get_driver_version(void);
> >
> > +#define VK_EXT_OFFSET (10UL)
> > +#define vk_enum_extension(__enum) \
> > +   ((__enum) >= VK_EXT_OFFSET ? __enum) - VK_EXT_OFFSET) /
> 1000UL) + 1) : 0)
> > +#define vk_enum_offset(__enum) \
> > +   ((__enum) >= VK_EXT_OFFSET ? ((__enum) % 1000) : (__enum))
> 
> The macro functions, when called, look like regular functions due to
> being lowercase. But they don't behave like functions; their arguments
> suffer from the multiple evaluation disease.
> 
> Please rename the macros to be all uppercase, so callers' expectations
> will be set correctly. Or, even better, define them as inline
> functions.
> 
> 
> +1 to static inline
> 
> Done locally.

Cool. Waiting to see it appear in wip/djeath/ycbcr_conversion.
> 
> 
> References:
> 
> [1] mailto:chadvers...@chromium.org
> [2] mailto:lionel.g.landwer...@intel.com

> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 01/11] vulkan: util: add macros to extract extension/offset number from enums

2017-10-03 Thread Lionel Landwerlin 

On 03/10/17 19:13, Jason Ekstrand wrote:
On Tue, Oct 3, 2017 at 9:43 AM, Chad Versace > wrote:


On Tue 03 Oct 2017, Lionel Landwerlin wrote:
> v2: Simplify offset enum computation (Jason)
>
> Signed-off-by: Lionel Landwerlin mailto:lionel.g.landwer...@intel.com>>
> ---
>  src/vulkan/util/vk_util.h | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/vulkan/util/vk_util.h b/src/vulkan/util/vk_util.h
> index 2ed601f881e..8c8cb64d513 100644
> --- a/src/vulkan/util/vk_util.h
> +++ b/src/vulkan/util/vk_util.h
> @@ -199,4 +199,10 @@ __vk_find_struct(void *start,
VkStructureType sType)
>
>  uint32_t vk_get_driver_version(void);
>
> +#define VK_EXT_OFFSET (10UL)
> +#define vk_enum_extension(__enum) \
> +   ((__enum) >= VK_EXT_OFFSET ? __enum) - VK_EXT_OFFSET) /
1000UL) + 1) : 0)
> +#define vk_enum_offset(__enum) \
> +   ((__enum) >= VK_EXT_OFFSET ? ((__enum) % 1000) : (__enum))

The macro functions, when called, look like regular functions due to
being lowercase. But they don't behave like functions; their arguments
suffer from the multiple evaluation disease.

Please rename the macros to be all uppercase, so callers' expectations
will be set correctly. Or, even better, define them as inline
functions.


+1 to static inline


Done locally.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 01/11] vulkan: util: add macros to extract extension/offset number from enums

2017-10-03 Thread Jason Ekstrand 
On Tue, Oct 3, 2017 at 9:43 AM, Chad Versace 
wrote:

> On Tue 03 Oct 2017, Lionel Landwerlin wrote:
> > v2: Simplify offset enum computation (Jason)
> >
> > Signed-off-by: Lionel Landwerlin 
> > ---
> >  src/vulkan/util/vk_util.h | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/src/vulkan/util/vk_util.h b/src/vulkan/util/vk_util.h
> > index 2ed601f881e..8c8cb64d513 100644
> > --- a/src/vulkan/util/vk_util.h
> > +++ b/src/vulkan/util/vk_util.h
> > @@ -199,4 +199,10 @@ __vk_find_struct(void *start, VkStructureType sType)
> >
> >  uint32_t vk_get_driver_version(void);
> >
> > +#define VK_EXT_OFFSET (10UL)
> > +#define vk_enum_extension(__enum) \
> > +   ((__enum) >= VK_EXT_OFFSET ? __enum) - VK_EXT_OFFSET) / 1000UL)
> + 1) : 0)
> > +#define vk_enum_offset(__enum) \
> > +   ((__enum) >= VK_EXT_OFFSET ? ((__enum) % 1000) : (__enum))
>
> The macro functions, when called, look like regular functions due to
> being lowercase. But they don't behave like functions; their arguments
> suffer from the multiple evaluation disease.
>
> Please rename the macros to be all uppercase, so callers' expectations
> will be set correctly. Or, even better, define them as inline
> functions.


+1 to static inline
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2 01/11] vulkan: util: add macros to extract extension/offset number from enums

2017-10-03 Thread Chad Versace 
On Tue 03 Oct 2017, Lionel Landwerlin wrote:
> v2: Simplify offset enum computation (Jason)
> 
> Signed-off-by: Lionel Landwerlin 
> ---
>  src/vulkan/util/vk_util.h | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/vulkan/util/vk_util.h b/src/vulkan/util/vk_util.h
> index 2ed601f881e..8c8cb64d513 100644
> --- a/src/vulkan/util/vk_util.h
> +++ b/src/vulkan/util/vk_util.h
> @@ -199,4 +199,10 @@ __vk_find_struct(void *start, VkStructureType sType)
>  
>  uint32_t vk_get_driver_version(void);
>  
> +#define VK_EXT_OFFSET (10UL)
> +#define vk_enum_extension(__enum) \
> +   ((__enum) >= VK_EXT_OFFSET ? __enum) - VK_EXT_OFFSET) / 1000UL) + 1) 
> : 0)
> +#define vk_enum_offset(__enum) \
> +   ((__enum) >= VK_EXT_OFFSET ? ((__enum) % 1000) : (__enum))

The macro functions, when called, look like regular functions due to
being lowercase. But they don't behave like functions; their arguments
suffer from the multiple evaluation disease.

Please rename the macros to be all uppercase, so callers' expectations
will be set correctly. Or, even better, define them as inline
functions.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2 01/11] vulkan: util: add macros to extract extension/offset number from enums

2017-10-03 Thread Lionel Landwerlin 
v2: Simplify offset enum computation (Jason)

Signed-off-by: Lionel Landwerlin 
---
 src/vulkan/util/vk_util.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/vulkan/util/vk_util.h b/src/vulkan/util/vk_util.h
index 2ed601f881e..8c8cb64d513 100644
--- a/src/vulkan/util/vk_util.h
+++ b/src/vulkan/util/vk_util.h
@@ -199,4 +199,10 @@ __vk_find_struct(void *start, VkStructureType sType)
 
 uint32_t vk_get_driver_version(void);
 
+#define VK_EXT_OFFSET (10UL)
+#define vk_enum_extension(__enum) \
+   ((__enum) >= VK_EXT_OFFSET ? __enum) - VK_EXT_OFFSET) / 1000UL) + 1) : 
0)
+#define vk_enum_offset(__enum) \
+   ((__enum) >= VK_EXT_OFFSET ? ((__enum) % 1000) : (__enum))
+
 #endif /* VK_UTIL_H */
-- 
2.14.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev