[Mesa-dev] [PATCH] i965: Replace *_generator::shader with is_glsl boolean.

Paul Berry stereotype441 at gmail.com
Wed Jan 22 12:00:01 PST 2014


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.

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.
---
 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;
-- 
1.8.5.3



More information about the mesa-dev mailing list