Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp

2019-09-16 Thread Caio Marcelo de Oliveira Filho 
> How about something simple like this instead:
> 
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp
> b/src/mesa/state_tracker
> index d6a0264..4f5acfd 100644
> --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> @@ -687,9 +687,14 @@ st_link_nir(struct gl_context *ctx,
>  * st_nir_preprocess.
>  */
> if (shader_program->data->spirv) {
> -  static const gl_nir_linker_options opts = {
> - true /*fill_parameters */
> -  };
> +  /* Note: this object could be static const but designated
> +   * initializers are not part of the C++ standard (allowed by GCC
> +   * but not MSVC.)
> +   */
> +  gl_nir_linker_options opts = { 0 };
> +
> +  opts.fill_parameters = true;
> +
>if (!gl_nir_link(ctx, shader_program, ))
>   return GL_FALSE;

That looks fine to me.


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

Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp

2019-09-12 Thread Brian Paul 

On 09/11/2019 03:06 PM, Ian Romanick wrote:

On 9/10/19 10:53 PM, Brian Paul wrote:

IIRC, designated initializers are not legal C++.
Fixes the MSVC build.

Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function")
---
  src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
b/src/mesa/state_tracker/st_glsl_to_nir.cpp
index 280a778..d6a0264 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx,
  */
 if (shader_program->data->spirv) {
static const gl_nir_linker_options opts = {
- .fill_parameters = true,
+ true /*fill_parameters */


Could we get a comment in the definition of gl_nir_linker_options to
remind people to either add options only to the end or double check all
of the places that initialize the structures?  If someone adds 'bool
do_foo_instead_of_bar' option at the beginning of that struct, it will
cause problems.


};
if (!gl_nir_link(ctx, shader_program, ))
   return GL_FALSE;





How about something simple like this instead:


diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
b/src/mesa/state_tracker

index d6a0264..4f5acfd 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -687,9 +687,14 @@ st_link_nir(struct gl_context *ctx,
 * st_nir_preprocess.
 */
if (shader_program->data->spirv) {
-  static const gl_nir_linker_options opts = {
- true /*fill_parameters */
-  };
+  /* Note: this object could be static const but designated
+   * initializers are not part of the C++ standard (allowed by GCC
+   * but not MSVC.)
+   */
+  gl_nir_linker_options opts = { 0 };
+
+  opts.fill_parameters = true;
+
   if (!gl_nir_link(ctx, shader_program, ))
  return GL_FALSE;


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

Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp

2019-09-11 Thread Ian Romanick 
On 9/10/19 10:53 PM, Brian Paul wrote:
> IIRC, designated initializers are not legal C++.
> Fixes the MSVC build.
> 
> Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function")
> ---
>  src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
> b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> index 280a778..d6a0264 100644
> --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> @@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx,
>  */
> if (shader_program->data->spirv) {
>static const gl_nir_linker_options opts = {
> - .fill_parameters = true,
> + true /*fill_parameters */

Could we get a comment in the definition of gl_nir_linker_options to
remind people to either add options only to the end or double check all
of the places that initialize the structures?  If someone adds 'bool
do_foo_instead_of_bar' option at the beginning of that struct, it will
cause problems.

>};
>if (!gl_nir_link(ctx, shader_program, ))
>   return GL_FALSE;
> 

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

Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp

2019-09-11 Thread Matt Turner 
On Tue, Sep 10, 2019 at 10:54 PM Brian Paul  wrote:
>
> IIRC, designated initializers are not legal C++.
> Fixes the MSVC build.
>
> Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function")
> ---
>  src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
> b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> index 280a778..d6a0264 100644
> --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
> +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
> @@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx,
>  */
> if (shader_program->data->spirv) {
>static const gl_nir_linker_options opts = {
> - .fill_parameters = true,
> + true /*fill_parameters */

Probably intended to have a space after /*
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp

2019-09-11 Thread Neha Bhende 
Looks good to me.
Reviewed-by: Neha Bhende 

Regards,
Neha


From: Brian Paul 
Sent: Wednesday, September 11, 2019 11:23 AM
To: mesa-dev@lists.freedesktop.org
Cc: Neha Bhende; Charmaine Lee; aio.olive...@intel.com
Subject: [PATCH] st/nir: fix illegal designated initializer in 
st_glsl_to_nir.cpp

IIRC, designated initializers are not legal C++.
Fixes the MSVC build.

Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function")
---
 src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
b/src/mesa/state_tracker/st_glsl_to_nir.cpp
index 280a778..d6a0264 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx,
 */
if (shader_program->data->spirv) {
   static const gl_nir_linker_options opts = {
- .fill_parameters = true,
+ true /*fill_parameters */
   };
   if (!gl_nir_link(ctx, shader_program, ))
  return GL_FALSE;
--
1.8.5.6

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

[Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp

2019-09-10 Thread Brian Paul 
IIRC, designated initializers are not legal C++.
Fixes the MSVC build.

Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function")
---
 src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp 
b/src/mesa/state_tracker/st_glsl_to_nir.cpp
index 280a778..d6a0264 100644
--- a/src/mesa/state_tracker/st_glsl_to_nir.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp
@@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx,
 */
if (shader_program->data->spirv) {
   static const gl_nir_linker_options opts = {
- .fill_parameters = true,
+ true /*fill_parameters */
   };
   if (!gl_nir_link(ctx, shader_program, ))
  return GL_FALSE;
-- 
1.8.5.6

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