[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:41:25 PDT 2015
On Wed, Oct 14, 2015 at 11:25:40AM +0300, Pohjolainen, Topi wrote:
> 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
Or more appropiately using ralloc_asprintf_append()...
> 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
> _______________________________________________
> 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