On Wed, 2015-09-30 at 18:41 -0700, Jason Ekstrand wrote:
> The way we deal with ARB program uniforms is basically the same in both the
> vec4 and the fs backend. This commit takes the best parts of both
> implementations and pulls the common code into a shared helper function.
> ---
> src/mesa/drivers/dri/i965/Makefile.sources | 1 +
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 10 +----
> src/mesa/drivers/dri/i965/brw_nir.h | 9 ++++
> src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp | 61
> ++++++++++++++++++++++++++
> src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 29 ++----------
> 5 files changed, 76 insertions(+), 34 deletions(-)
> create mode 100644 src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
> b/src/mesa/drivers/dri/i965/Makefile.sources
> index 540e3df..eb8196d 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -86,6 +86,7 @@ i965_FILES = \
> brw_nir.h \
> brw_nir.c \
> brw_nir_analyze_boolean_resolves.c \
> + brw_nir_uniforms.cpp \
> brw_object_purgeable.c \
> brw_packed_float.c \
> brw_performance_monitor.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index d33e452..3ba6a67 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -196,14 +196,8 @@ fs_visitor::nir_setup_uniforms(nir_shader *shader)
> param_size[var->data.driver_location] =
> type_size_scalar(var->type);
> }
> } else {
> - /* prog_to_nir only creates a single giant uniform variable so we can
> - * just set param up directly. */
> - for (unsigned p = 0; p < prog->Parameters->NumParameters; p++) {
> - for (unsigned int i = 0; i < 4; i++) {
> - stage_prog_data->param[4 * p + i] =
> - &prog->Parameters->ParameterValues[p][i];
> - }
> - }
> + brw_nir_setup_arb_uniforms(shader, prog, stage_prog_data);
> +
> if(prog->Parameters->NumParameters > 0)
> param_size[0] = prog->Parameters->NumParameters * 4;
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.h
> b/src/mesa/drivers/dri/i965/brw_nir.h
> index ad71293..b4a6dc0 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.h
> +++ b/src/mesa/drivers/dri/i965/brw_nir.h
> @@ -85,6 +85,15 @@ enum brw_reg_type brw_type_for_nir_type(nir_alu_type type);
>
> enum glsl_base_type brw_glsl_base_type_for_nir_type(nir_alu_type type);
>
> +void brw_nir_setup_glsl_uniforms(nir_shader *shader,
> + struct gl_shader_program *shader_prog,
> + const struct gl_program *prog,
> + struct brw_stage_prog_data *stage_prog_data,
> + bool is_scalar);
brw_nir_setup_glsl_uniforms should be declared in the next patch.
> +void brw_nir_setup_arb_uniforms(nir_shader *shader, struct gl_program *prog,
> + struct brw_stage_prog_data *stage_prog_data);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> new file mode 100644
> index 0000000..ede4e91
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
> @@ -0,0 +1,61 @@
> +/*
> + * Copyright © 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "brw_shader.h"
> +#include "brw_nir.h"
> +
> +void
> +brw_nir_setup_arb_uniforms(nir_shader *shader, struct gl_program *prog,
> + struct brw_stage_prog_data *stage_prog_data)
> +{
> + struct gl_program_parameter_list *plist = prog->Parameters;
> +
> +#ifndef NDEBUG
> + if (!shader->uniforms.is_empty()) {
> + /* For ARB programs, only a single "parameters" variable is generated
> to
> + * support uniform data.
> + */
> + assert(shader->uniforms.length() == 1);
> + nir_variable *var = (nir_variable *) shader->uniforms.get_head();
> + assert(strcmp(var->name, "parameters") == 0);
> + assert(var->type->array_size() == (int)plist->NumParameters);
> + }
> +#endif
> +
> + for (unsigned p = 0; p < plist->NumParameters; p++) {
> + /* Parameters should be either vec4 uniforms or single component
> + * constants; matrices and other larger types should have been broken
> + * down earlier.
> + */
> + assert(plist->Parameters[p].Size <= 4);
> +
> + unsigned i;
> + for (i = 0; i < plist->Parameters[p].Size; i++) {
> + stage_prog_data->param[4 * p + i] = &plist->ParameterValues[p][i];
> + }
> + for (; i < 4; i++) {
> + static const gl_constant_value zero = { 0.0 };
> + stage_prog_data->param[4 * p + i] = &zero;
> + }
> + }
> +}
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> index ee94e58..99fd71f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> @@ -153,33 +153,10 @@ vec4_visitor::nir_setup_uniforms(nir_shader *shader)
> nir_setup_uniform(var);
> }
> } else {
> - /* For ARB_vertex_program, only a single "parameters" variable is
> - * generated to support uniform data.
> - */
> - nir_variable *var = (nir_variable *) shader->uniforms.get_head();
> - assert(shader->uniforms.length() == 1 &&
> - strcmp(var->name, "parameters") == 0);
> -
> - assert(var->data.driver_location == 0);
> - uniform_size[0] = type_size_vec4(var->type);
> -
> - struct gl_program_parameter_list *plist = prog->Parameters;
> - for (unsigned p = 0; p < plist->NumParameters; p++) {
> - /* Parameters should be either vec4 uniforms or single component
> - * constants; matrices and other larger types should have been
> broken
> - * down earlier.
> - */
> - assert(plist->Parameters[p].Size <= 4);
> + brw_nir_setup_arb_uniforms(shader, prog, stage_prog_data);
>
> - unsigned i;
> - for (i = 0; i < plist->Parameters[p].Size; i++) {
> - stage_prog_data->param[p * 4 + i] =
> &plist->ParameterValues[p][i];
> - }
> - for (; i < 4; i++) {
> - static const gl_constant_value zero = { 0.0 };
> - stage_prog_data->param[p * 4 + i] = &zero;
> - }
> - }
> + if(prog->Parameters->NumParameters > 0)
> + uniform_size[0] = prog->Parameters->NumParameters;
Feel free to ignore this, but I wonder: why don't we just remove the
condition? (here and in the FS backend). It does not seem like it helps
anything, right?
With the comment about brw_nir_setup_glsl_uniforms addressed, this is:
Reviewed-by: Iago Toral Quiroga <ito...@igalia.com>
> }
> }
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
