Acked-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> <div class="gmail_quote">On Mar 6, 2015 2:18 AM, "Kenneth Graunke" <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> Cc: Eric Anholt <<a href="mailto:eric@anholt.net">eric@anholt.net</a>> --- 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-><a href="http://base.ir" target="_blank">base.ir</a>); - nir_shader *nir = glsl_to_nir(shader-><a href="http://base.ir" target="_blank">base.ir</a>, NULL, true); + nir_shader *nir = glsl_to_nir(shader-><a href="http://base.ir" target="_blank">base.ir</a>, 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 <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>