[Mesa-dev] [PATCH] i965: Replace *_generator::shader with is_glsl boolean.
Kenneth Graunke
kenneth at whitecape.org
Wed Jan 22 15:24:10 PST 2014
On 01/22/2014 12:00 PM, Paul Berry wrote:
> The "shader" field in fs_generator, vec4_generator, and gen8_generator
> was only used for one purpose; to figure out if we were compiling an
> assembly shader or a GLSL shader. And it wasn't being used properly:
> in vec4 shaders we were always initializing it based on
> prog->_LinkedShaders[MESA_SHADER_FRAGMENT], regardless of whether we
> were compiling a geometry shader or a vertex shader.
I think you meant to say MESA_SHADER_VERTEX here. It definitely never
uses MESA_SHADER_FRAGMENT for VS/GS programs.
> This was a fairly benign problem, since it's unlikely that a
> real-world program will try to mix and match GLSL and assembly shaders
> using separate shader objects. But it seems worth fixing.
>
> This patch replaces the shader field with a new is_glsl boolean, and
> initializes it based on information from the caller, so that it always
> refers to the correct shader stage.
I don't think you need this new boolean. I'm pretty sure that you can
replace shader != NULL with shader_prog != NULL and achieve the same
results.
In the long term, I really would like to see some unification between
gl_shader_program and gl_program. We always meant to do that, but it
kind of died with Ian's "let's convert ARB programs to GLSL IR" idea.
At that point, it'd probably make sense to store a "generated from
GLSL/ARB/FF" indicator there. It'd be nicer than passing extra flags
along with the structures...
Obviously, that's down the road though. Would you be willing to try the
shader_prog != NULL idea?
> ---
> src/mesa/drivers/dri/i965/brw_fs.cpp | 6 ++++--
> src/mesa/drivers/dri/i965/brw_fs.h | 8 +++++---
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 12 ++++++------
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 4 ++--
> src/mesa/drivers/dri/i965/brw_vec4.h | 8 +++++---
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 11 +++++------
> src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 7 +++++--
> src/mesa/drivers/dri/i965/gen8_fs_generator.cpp | 13 ++++++-------
> src/mesa/drivers/dri/i965/gen8_generator.cpp | 6 ++++--
> src/mesa/drivers/dri/i965/gen8_generator.h | 5 +++--
> src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp | 10 +++++-----
> 11 files changed, 50 insertions(+), 40 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index a0e4830..c0d65d5 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3512,11 +3512,13 @@ brw_wm_fs_emit(struct brw_context *brw, struct brw_wm_compile *c,
>
> const unsigned *assembly = NULL;
> if (brw->gen >= 8) {
> - gen8_fs_generator g(brw, c, prog, fp, v.dual_src_output.file != BAD_FILE);
> + gen8_fs_generator g(brw, c, prog, fp, v.dual_src_output.file != BAD_FILE,
> + shader != NULL);
> assembly = g.generate_assembly(&v.instructions, simd16_instructions,
> final_assembly_size);
> } else {
> - fs_generator g(brw, c, prog, fp, v.dual_src_output.file != BAD_FILE);
> + fs_generator g(brw, c, prog, fp, v.dual_src_output.file != BAD_FILE,
> + shader != NULL);
> assembly = g.generate_assembly(&v.instructions, simd16_instructions,
> final_assembly_size);
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index a903908..ad0aa99 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -512,7 +512,8 @@ public:
> struct brw_wm_compile *c,
> struct gl_shader_program *prog,
> struct gl_fragment_program *fp,
> - bool dual_source_output);
> + bool dual_source_output,
> + bool is_glsl);
> ~fs_generator();
>
> const unsigned *generate_assembly(exec_list *simd8_instructions,
> @@ -615,7 +616,6 @@ private:
> struct brw_wm_compile *c;
>
> struct gl_shader_program *prog;
> - struct gl_shader *shader;
> const struct gl_fragment_program *fp;
>
> unsigned dispatch_width; /**< 8 or 16 */
> @@ -623,6 +623,7 @@ private:
> exec_list discard_halt_patches;
> bool dual_source_output;
> void *mem_ctx;
> + const bool is_glsl;
> };
>
> /**
> @@ -637,7 +638,8 @@ public:
> struct brw_wm_compile *c,
> struct gl_shader_program *prog,
> struct gl_fragment_program *fp,
> - bool dual_source_output);
> + bool dual_source_output,
> + bool is_glsl);
> ~gen8_fs_generator();
>
> const unsigned *generate_assembly(exec_list *simd8_instructions,
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index e701fc5..a8e81b8 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -40,14 +40,14 @@ fs_generator::fs_generator(struct brw_context *brw,
> struct brw_wm_compile *c,
> struct gl_shader_program *prog,
> struct gl_fragment_program *fp,
> - bool dual_source_output)
> + bool dual_source_output,
> + bool is_glsl)
>
> - : brw(brw), c(c), prog(prog), fp(fp), dual_source_output(dual_source_output)
> + : brw(brw), c(c), prog(prog), fp(fp),
> + dual_source_output(dual_source_output), is_glsl(is_glsl)
> {
> ctx = &brw->ctx;
>
> - shader = prog ? prog->_LinkedShaders[MESA_SHADER_FRAGMENT] : NULL;
> -
> mem_ctx = c;
>
> p = rzalloc(mem_ctx, struct brw_compile);
> @@ -1301,7 +1301,7 @@ fs_generator::generate_code(exec_list *instructions)
> const void *last_annotation_ir = NULL;
>
> if (unlikely(INTEL_DEBUG & DEBUG_WM)) {
> - if (shader) {
> + if (is_glsl) {
> printf("Native code for fragment shader %d (SIMD%d dispatch):\n",
> prog->Name, dispatch_width);
> } else if (fp) {
> @@ -1342,7 +1342,7 @@ fs_generator::generate_code(exec_list *instructions)
> last_annotation_ir = inst->ir;
> if (last_annotation_ir) {
> printf(" ");
> - if (shader)
> + if (is_glsl)
> ((ir_instruction *)inst->ir)->print();
> else {
> const prog_instruction *fpi;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index f4f4019..e7a7dca 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1679,11 +1679,11 @@ brw_vs_emit(struct brw_context *brw,
> const unsigned *assembly = NULL;
> if (brw->gen >= 8) {
> gen8_vec4_generator g(brw, prog, &c->vp->program.Base, &prog_data->base,
> - mem_ctx, INTEL_DEBUG & DEBUG_VS);
> + mem_ctx, INTEL_DEBUG & DEBUG_VS, shader != NULL);
> assembly = g.generate_assembly(&v.instructions, final_assembly_size);
> } else {
> vec4_generator g(brw, prog, &c->vp->program.Base, &prog_data->base,
> - mem_ctx, INTEL_DEBUG & DEBUG_VS);
> + mem_ctx, INTEL_DEBUG & DEBUG_VS, shader != NULL);
> assembly = g.generate_assembly(&v.instructions, final_assembly_size);
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 4a5b577..5e0bb97 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -571,7 +571,8 @@ public:
> struct gl_program *prog,
> struct brw_vec4_prog_data *prog_data,
> void *mem_ctx,
> - bool debug_flag);
> + bool debug_flag,
> + bool is_glsl);
> ~vec4_generator();
>
> const unsigned *generate_assembly(exec_list *insts, unsigned *asm_size);
> @@ -652,13 +653,13 @@ private:
> struct brw_compile *p;
>
> struct gl_shader_program *shader_prog;
> - struct gl_shader *shader;
> const struct gl_program *prog;
>
> struct brw_vec4_prog_data *prog_data;
>
> void *mem_ctx;
> const bool debug_flag;
> + const bool is_glsl;
> };
>
> /**
> @@ -674,7 +675,8 @@ public:
> struct gl_program *prog,
> struct brw_vec4_prog_data *prog_data,
> void *mem_ctx,
> - bool debug_flag);
> + bool debug_flag,
> + bool is_glsl);
> ~gen8_vec4_generator();
>
> const unsigned *generate_assembly(exec_list *insts, unsigned *asm_size);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 51e88d2..cf4a2c6 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -138,12 +138,11 @@ vec4_generator::vec4_generator(struct brw_context *brw,
> struct gl_program *prog,
> struct brw_vec4_prog_data *prog_data,
> void *mem_ctx,
> - bool debug_flag)
> + bool debug_flag,
> + bool is_glsl)
> : brw(brw), shader_prog(shader_prog), prog(prog), prog_data(prog_data),
> - mem_ctx(mem_ctx), debug_flag(debug_flag)
> + mem_ctx(mem_ctx), debug_flag(debug_flag), is_glsl(is_glsl)
> {
> - shader = shader_prog ? shader_prog->_LinkedShaders[MESA_SHADER_VERTEX] : NULL;
> -
> p = rzalloc(mem_ctx, struct brw_compile);
> brw_init_compile(brw, p, mem_ctx);
> }
> @@ -1235,7 +1234,7 @@ vec4_generator::generate_code(exec_list *instructions)
> const void *last_annotation_ir = NULL;
>
> if (unlikely(debug_flag)) {
> - if (shader) {
> + if (is_glsl) {
> printf("Native code for vertex shader %d:\n", shader_prog->Name);
> } else {
> printf("Native code for vertex program %d:\n", prog->Id);
> @@ -1251,7 +1250,7 @@ vec4_generator::generate_code(exec_list *instructions)
> last_annotation_ir = inst->ir;
> if (last_annotation_ir) {
> printf(" ");
> - if (shader) {
> + if (is_glsl) {
> ((ir_instruction *) last_annotation_ir)->print();
> } else {
> const prog_instruction *vpi;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> index 40743cc..c183ee8 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> @@ -550,13 +550,16 @@ generate_assembly(struct brw_context *brw,
> exec_list *instructions,
> unsigned *final_assembly_size)
> {
> + /* We don't support assembly geometry shaders */
> + const bool is_glsl = true;
> +
> if (brw->gen >= 8) {
> gen8_vec4_generator g(brw, shader_prog, prog, prog_data, mem_ctx,
> - INTEL_DEBUG & DEBUG_GS);
> + INTEL_DEBUG & DEBUG_GS, is_glsl);
> return g.generate_assembly(instructions, final_assembly_size);
> } else {
> vec4_generator g(brw, shader_prog, prog, prog_data, mem_ctx,
> - INTEL_DEBUG & DEBUG_GS);
> + INTEL_DEBUG & DEBUG_GS, is_glsl);
> return g.generate_assembly(instructions, final_assembly_size);
> }
> }
> diff --git a/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp b/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp
> index febd425..ce1cff9 100644
> --- a/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/gen8_fs_generator.cpp
> @@ -39,12 +39,11 @@ gen8_fs_generator::gen8_fs_generator(struct brw_context *brw,
> struct brw_wm_compile *c,
> struct gl_shader_program *shader_prog,
> struct gl_fragment_program *fp,
> - bool dual_source_output)
> - : gen8_generator(brw, shader_prog, fp ? &fp->Base : NULL, c), c(c), fp(fp),
> - dual_source_output(dual_source_output)
> + bool dual_source_output,
> + bool is_glsl)
> + : gen8_generator(brw, shader_prog, fp ? &fp->Base : NULL, c, is_glsl), c(c),
> + fp(fp), dual_source_output(dual_source_output)
> {
> - shader =
> - shader_prog ? shader_prog->_LinkedShaders[MESA_SHADER_FRAGMENT] : NULL;
> }
>
> gen8_fs_generator::~gen8_fs_generator()
> @@ -567,7 +566,7 @@ gen8_fs_generator::generate_code(exec_list *instructions)
> const void *last_annotation_ir = NULL;
>
> if (unlikely(INTEL_DEBUG & DEBUG_WM)) {
> - if (shader) {
> + if (is_glsl) {
> printf("Native code for fragment shader %d (SIMD%d dispatch):\n",
> shader_prog->Name, dispatch_width);
> } else if (fp) {
> @@ -608,7 +607,7 @@ gen8_fs_generator::generate_code(exec_list *instructions)
> last_annotation_ir = ir->ir;
> if (last_annotation_ir) {
> printf(" ");
> - if (shader) {
> + if (is_glsl) {
> ((ir_instruction *) ir->ir)->print();
> } else if (prog) {
> const prog_instruction *fpi;
> diff --git a/src/mesa/drivers/dri/i965/gen8_generator.cpp b/src/mesa/drivers/dri/i965/gen8_generator.cpp
> index ee5f792..de4708a 100644
> --- a/src/mesa/drivers/dri/i965/gen8_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/gen8_generator.cpp
> @@ -40,8 +40,10 @@ extern "C" {
> gen8_generator::gen8_generator(struct brw_context *brw,
> struct gl_shader_program *shader_prog,
> struct gl_program *prog,
> - void *mem_ctx)
> - : shader_prog(shader_prog), prog(prog), brw(brw), mem_ctx(mem_ctx)
> + void *mem_ctx,
> + bool is_glsl)
> + : shader_prog(shader_prog), prog(prog), brw(brw), mem_ctx(mem_ctx),
> + is_glsl(is_glsl)
> {
> ctx = &brw->ctx;
>
> diff --git a/src/mesa/drivers/dri/i965/gen8_generator.h b/src/mesa/drivers/dri/i965/gen8_generator.h
> index 7d74267..7fdb468 100644
> --- a/src/mesa/drivers/dri/i965/gen8_generator.h
> +++ b/src/mesa/drivers/dri/i965/gen8_generator.h
> @@ -40,7 +40,8 @@ public:
> gen8_generator(struct brw_context *brw,
> struct gl_shader_program *shader_prog,
> struct gl_program *prog,
> - void *mem_ctx);
> + void *mem_ctx,
> + bool is_glsl);
> ~gen8_generator();
>
> /**
> @@ -133,7 +134,6 @@ protected:
> gen8_instruction *next_inst(unsigned opcode);
>
> struct gl_shader_program *shader_prog;
> - struct gl_shader *shader;
> struct gl_program *prog;
>
> struct brw_context *brw;
> @@ -195,4 +195,5 @@ protected:
> } default_state;
>
> void *mem_ctx;
> + const bool is_glsl;
> };
> diff --git a/src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp b/src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp
> index ee86dbb..fcff71b 100644
> --- a/src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/gen8_vec4_generator.cpp
> @@ -37,12 +37,12 @@ gen8_vec4_generator::gen8_vec4_generator(struct brw_context *brw,
> struct gl_program *prog,
> struct brw_vec4_prog_data *prog_data,
> void *mem_ctx,
> - bool debug_flag)
> - : gen8_generator(brw, shader_prog, prog, mem_ctx),
> + bool debug_flag,
> + bool is_glsl)
> + : gen8_generator(brw, shader_prog, prog, mem_ctx, is_glsl),
> prog_data(prog_data),
> debug_flag(debug_flag)
> {
> - shader = shader_prog ? shader_prog->_LinkedShaders[MESA_SHADER_VERTEX] : NULL;
> }
>
> gen8_vec4_generator::~gen8_vec4_generator()
> @@ -783,7 +783,7 @@ gen8_vec4_generator::generate_code(exec_list *instructions)
> const void *last_annotation_ir = NULL;
>
> if (unlikely(debug_flag)) {
> - if (shader) {
> + if (is_glsl) {
> printf("Native code for vertex shader %d:\n", shader_prog->Name);
> } else {
> printf("Native code for vertex program %d:\n", prog->Id);
> @@ -799,7 +799,7 @@ gen8_vec4_generator::generate_code(exec_list *instructions)
> last_annotation_ir = ir->ir;
> if (last_annotation_ir) {
> printf(" ");
> - if (shader) {
> + if (is_glsl) {
> ((ir_instruction *) last_annotation_ir)->print();
> } else {
> const prog_instruction *vpi;
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140122/6b5c3e3e/attachment-0001.pgp>
More information about the mesa-dev
mailing list