[Mesa-dev] [PATCH v2 12/17] i965/vs: Rework vs_emit to take a nir_shader and a brw_compiler
Pohjolainen, Topi
topi.pohjolainen at intel.com
Wed Oct 14 01:25:40 PDT 2015
On Sat, Oct 10, 2015 at 08:09:01AM -0700, Jason Ekstrand wrote:
> This commit removes all dependence on GL state by getting rid of the
> brw_context parameter and the GL data structures.
>
> v2 (Jason Ekstrand):
> - Patch use_legacy_snorm_formula through as a function argument rather
> than trying to go through the shader key.
> ---
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 70 +++++++++++++---------------------
> src/mesa/drivers/dri/i965/brw_vs.c | 16 +++++++-
> src/mesa/drivers/dri/i965/brw_vs.h | 12 ++++--
> 3 files changed, 49 insertions(+), 49 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 4b8390f..8e38729 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1937,51 +1937,42 @@ extern "C" {
> * Returns the final assembly and the program's size.
> */
> const unsigned *
> -brw_vs_emit(struct brw_context *brw,
> +brw_vs_emit(const struct brw_compiler *compiler, void *log_data,
> void *mem_ctx,
> const struct brw_vs_prog_key *key,
> struct brw_vs_prog_data *prog_data,
> - struct gl_vertex_program *vp,
> - struct gl_shader_program *prog,
> + const nir_shader *shader,
> + gl_clip_plane *clip_planes,
> + bool use_legacy_snorm_formula,
> int shader_time_index,
> - unsigned *final_assembly_size)
> + unsigned *final_assembly_size,
> + char **error_str)
> {
> const unsigned *assembly = NULL;
>
> - if (brw->intelScreen->compiler->scalar_vs) {
> + if (compiler->scalar_vs) {
> prog_data->base.dispatch_mode = DISPATCH_MODE_SIMD8;
>
> - fs_visitor v(brw->intelScreen->compiler, brw,
> - mem_ctx, key, &prog_data->base.base,
> + fs_visitor v(compiler, log_data, mem_ctx, key, &prog_data->base.base,
> NULL, /* prog; Only used for TEXTURE_RECTANGLE on gen < 8 */
> - vp->Base.nir, 8, shader_time_index);
> - if (!v.run_vs(brw_select_clip_planes(&brw->ctx))) {
> - if (prog) {
> - prog->LinkStatus = false;
> - ralloc_strcat(&prog->InfoLog, v.fail_msg);
> - }
> -
> - _mesa_problem(NULL, "Failed to compile vertex shader: %s\n",
> - v.fail_msg);
> + shader, 8, shader_time_index);
> + if (!v.run_vs(clip_planes)) {
> + if (error_str)
> + *error_str = ralloc_strdup(mem_ctx, v.fail_msg);
I don't particularly like the complexity of the error reporting mechanism.
First vec4_visitor::fail() uses ralloc_asprintf() to create one string, then
we make a copy of it here and finally the caller of brw_vs_emit() makes yet
another copy using ralloc_strcat().
I wonder if we could pass the final destination all the way for the
vec4_visitor::fail() to augment with ralloc_asprintf() and hence avoid all
the indirection in the middle. What do you think?
>
> return NULL;
> }
>
> - fs_generator g(brw->intelScreen->compiler, brw,
> - mem_ctx, (void *) key, &prog_data->base.base,
> - v.promoted_constants,
> + fs_generator g(compiler, log_data, mem_ctx, (void *) key,
> + &prog_data->base.base, v.promoted_constants,
> v.runtime_check_aads_emit, "VS");
> if (INTEL_DEBUG & DEBUG_VS) {
> - char *name;
> - if (prog) {
> - name = ralloc_asprintf(mem_ctx, "%s vertex shader %d",
> - prog->Label ? prog->Label : "unnamed",
> - prog->Name);
> - } else {
> - name = ralloc_asprintf(mem_ctx, "vertex program %d",
> - vp->Base.Id);
> - }
> - g.enable_debug(name);
> + const char *debug_name =
> + ralloc_asprintf(mem_ctx, "%s vertex shader %s",
> + shader->info.label ? shader->info.label : "unnamed",
> + shader->info.name);
> +
> + g.enable_debug(debug_name);
> }
> g.generate_code(v.cfg, 8);
> assembly = g.get_assembly(final_assembly_size);
> @@ -1990,26 +1981,19 @@ brw_vs_emit(struct brw_context *brw,
> if (!assembly) {
> prog_data->base.dispatch_mode = DISPATCH_MODE_4X2_DUAL_OBJECT;
>
> - vec4_vs_visitor v(brw->intelScreen->compiler, brw, key, prog_data,
> - vp->Base.nir, brw_select_clip_planes(&brw->ctx),
> - mem_ctx, shader_time_index,
> - !_mesa_is_gles3(&brw->ctx));
> + vec4_vs_visitor v(compiler, log_data, key, prog_data,
> + shader, clip_planes, mem_ctx,
> + shader_time_index, use_legacy_snorm_formula);
> if (!v.run()) {
> - if (prog) {
> - prog->LinkStatus = false;
> - ralloc_strcat(&prog->InfoLog, v.fail_msg);
> - }
> -
> - _mesa_problem(NULL, "Failed to compile vertex shader: %s\n",
> - v.fail_msg);
> + if (error_str)
> + *error_str = ralloc_strdup(mem_ctx, v.fail_msg);
>
> return NULL;
> }
>
> - vec4_generator g(brw->intelScreen->compiler, brw,
> - &prog_data->base,
> + vec4_generator g(compiler, log_data, &prog_data->base,
> mem_ctx, INTEL_DEBUG & DEBUG_VS, "vertex", "VS");
> - assembly = g.generate_assembly(v.cfg, final_assembly_size, vp->Base.nir);
> + assembly = g.generate_assembly(v.cfg, final_assembly_size, shader);
> }
>
> return assembly;
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index 38de98f..abf744c 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -31,6 +31,7 @@
>
>
> #include "main/compiler.h"
> +#include "main/context.h"
> #include "brw_context.h"
> #include "brw_vs.h"
> #include "brw_util.h"
> @@ -179,9 +180,20 @@ brw_codegen_vs_prog(struct brw_context *brw,
>
> /* Emit GEN4 code.
> */
> - program = brw_vs_emit(brw, mem_ctx, key, &prog_data,
> - &vp->program, prog, st_index, &program_size);
> + char *error_str;
> + program = brw_vs_emit(brw->intelScreen->compiler, brw, mem_ctx, key,
> + &prog_data, vp->program.Base.nir,
> + brw_select_clip_planes(&brw->ctx),
> + !_mesa_is_gles3(&brw->ctx),
> + st_index, &program_size, &error_str);
> if (program == NULL) {
> + if (prog) {
> + prog->LinkStatus = false;
> + ralloc_strcat(&prog->InfoLog, error_str);
> + }
> +
> + _mesa_problem(NULL, "Failed to compile vertex shader: %s\n", error_str);
> +
> ralloc_free(mem_ctx);
> return false;
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.h b/src/mesa/drivers/dri/i965/brw_vs.h
> index c927cac..b65dd3b 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.h
> +++ b/src/mesa/drivers/dri/i965/brw_vs.h
> @@ -54,14 +54,18 @@
> extern "C" {
> #endif
>
> -const unsigned *brw_vs_emit(struct brw_context *brw,
> +struct nir_shader;
> +
> +const unsigned *brw_vs_emit(const struct brw_compiler *compiler, void *log_data,
> void *mem_ctx,
> const struct brw_vs_prog_key *key,
> struct brw_vs_prog_data *prog_data,
> - struct gl_vertex_program *vp,
> - struct gl_shader_program *shader_prog,
> + const struct nir_shader *shader,
> + gl_clip_plane *clip_planes,
> + bool use_legacy_snorm_formula,
> int shader_time_index,
> - unsigned *program_size);
> + unsigned *final_assembly_size,
> + char **error_str);
> void brw_vs_debug_recompile(struct brw_context *brw,
> struct gl_shader_program *prog,
> const struct brw_vs_prog_key *key);
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list