Acked-by: Jason Ekstrand <jason.ekstr...@intel.com>
On Mar 6, 2015 2:18 AM, "Kenneth Graunke" <kenn...@whitecape.org> wrote:
> The code in glsl_to_nir is entirely dead, as we translate from GLSL to
> NIR at link time, when there isn't a _mesa_glsl_parse_state to pass,
> so every caller passes NULL.
>
> glsl_to_nir seems like the wrong place to try and create the shader
> compiler options structure anyway - tgsi_to_nir, prog_to_nir, and other
> translators all would have to duplicate that code. The driver should
> set this up once with whatever settings it wants, and pass it in.
>
> Eric also added a NirOptions field to ctx->Const.ShaderCompilerOptions[]
> and left a comment saying: "The memory for the options is expected to be
> kept in a single static copy by the driver." This suggests the plan was
> to do exactly that. That pointer was not marked const, however, and the
> dead code used a mix of static structures and ralloced ones.
>
> This patch deletes the dead code in glsl_to_nir, instead making it take
> the shader compiler options as a mandatory argument. It creates an
> (empty) options struct in the i965 driver, and makes NirOptions point
> to that. It marks the pointer const so that we can actually do so
> without generating "discards const qualifier" compiler warnings.
>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> Cc: Eric Anholt <e...@anholt.net>
> ---
> src/glsl/nir/glsl_to_nir.cpp | 28 ++--------------------------
> src/glsl/nir/glsl_to_nir.h | 4 ++--
> src/mesa/drivers/dri/i965/brw_context.c | 5 +++++
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 5 ++++-
> src/mesa/main/mtypes.h | 2 +-
> 5 files changed, 14 insertions(+), 30 deletions(-)
>
> Eric, does this look reasonable to you?
>
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index b82e5c7..7e40ef4 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -124,34 +124,10 @@ private:
>
> }; /* end of anonymous namespace */
>
> -static const nir_shader_compiler_options default_options = {
> -};
> -
> nir_shader *
> -glsl_to_nir(exec_list *ir, _mesa_glsl_parse_state *state,
> - bool native_integers)
> +glsl_to_nir(exec_list *ir, bool native_integers,
> + const nir_shader_compiler_options *options)
> {
> - const nir_shader_compiler_options *options;
> -
> - if (state) {
> - struct gl_context *ctx = state->ctx;
> - struct gl_shader_compiler_options *gl_options =
> - &ctx->Const.ShaderCompilerOptions[state->stage];
> -
> - if (!gl_options->NirOptions) {
> - nir_shader_compiler_options *new_options =
> - rzalloc(ctx, nir_shader_compiler_options);
> - options = gl_options->NirOptions = new_options;
> -
> - if (gl_options->EmitNoPow)
> - new_options->lower_fpow = true;
> - } else {
> - options = gl_options->NirOptions;
> - }
> - } else {
> - options = &default_options;
> - }
> -
> nir_shader *shader = nir_shader_create(NULL, options);
>
> nir_visitor v1(shader, native_integers);
> diff --git a/src/glsl/nir/glsl_to_nir.h b/src/glsl/nir/glsl_to_nir.h
> index 58b2cee..7300945 100644
> --- a/src/glsl/nir/glsl_to_nir.h
> +++ b/src/glsl/nir/glsl_to_nir.h
> @@ -32,8 +32,8 @@
> extern "C" {
> #endif
>
> -nir_shader *glsl_to_nir(exec_list * ir, _mesa_glsl_parse_state *state,
> - bool native_integers);
> +nir_shader *glsl_to_nir(exec_list *ir, bool native_integers,
> + const nir_shader_compiler_options *options);
>
> #ifdef __cplusplus
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 786e6f5..892d4ca 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -68,6 +68,8 @@
> #include "tnl/t_pipeline.h"
> #include "util/ralloc.h"
>
> +#include "glsl/nir/nir.h"
> +
> /***************************************
> * Mesa's Driver Functions
> ***************************************/
> @@ -549,6 +551,8 @@ brw_initialize_context_constants(struct brw_context
> *brw)
> ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxInputComponents = 128;
> }
>
> + static const nir_shader_compiler_options nir_options = {};
> +
> /* We want the GLSL compiler to emit code that uses condition codes */
> for (int i = 0; i < MESA_SHADER_STAGES; i++) {
> ctx->Const.ShaderCompilerOptions[i].MaxIfDepth = brw->gen < 6 ? 16
> : UINT_MAX;
> @@ -562,6 +566,7 @@ brw_initialize_context_constants(struct brw_context
> *brw)
> (i == MESA_SHADER_FRAGMENT);
> ctx->Const.ShaderCompilerOptions[i].EmitNoIndirectUniform = false;
> ctx->Const.ShaderCompilerOptions[i].LowerClipDistance = true;
> + ctx->Const.ShaderCompilerOptions[i].NirOptions = &nir_options;
> }
>
> ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].OptimizeForAOS =
> true;
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index a0300aa..e24bf92 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -82,9 +82,12 @@ count_nir_instrs(nir_shader *nir)
> void
> fs_visitor::emit_nir_code()
> {
> + const nir_shader_compiler_options *options =
> + ctx->Const.ShaderCompilerOptions[stage].NirOptions;
> +
> /* first, lower the GLSL IR shader to NIR */
> lower_output_reads(shader->base.ir);
> - nir_shader *nir = glsl_to_nir(shader->base.ir, NULL, true);
> + nir_shader *nir = glsl_to_nir(shader->base.ir, true, options);
> nir_validate_shader(nir);
>
> nir_lower_global_vars_to_local(nir);
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index efeee8b..c43c6ac 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3036,7 +3036,7 @@ struct gl_shader_compiler_options
>
> struct gl_sl_pragmas DefaultPragmas; /**< Default #pragma settings */
>
> - struct nir_shader_compiler_options *NirOptions;
> + const struct nir_shader_compiler_options *NirOptions;
> };
>
>
> --
> 2.2.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
