[Mesa-dev] [PATCH v2 09/19] i965/vs: Make some vec4_visitor functions virtual.

Kenneth Graunke kenneth at whitecape.org
Wed Apr 10 13:17:06 PDT 2013


On 04/09/2013 03:11 PM, Paul Berry wrote:
> This patch makes the following vec4_visitor functions virtual, since
> they will need to be implemented differently for vertex and geometry
> shaders.  Some of the functions are renamed to reflect their generic
> purpose, rather than their VS-specific behaviour:
>
> - setup_attributes
> - emit_attribute_fixups (renamed to emit_prolog)
> - emit_vertex_program_code (renamed to emit_program_code)
> - emit_urb_writes (renamed to emit_thread_end)
>
> Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
> Reviewed-by: Eric Anholt <eric at anholt.net>
> ---
>   src/mesa/drivers/dri/i965/brw_vec4.cpp             |  8 ++++----
>   src/mesa/drivers/dri/i965/brw_vec4.h               | 16 ++++++++++++----
>   src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp     |  4 ++--
>   src/mesa/drivers/dri/i965/brw_vec4_vp.cpp          |  2 +-
>   .../dri/i965/test_vec4_register_coalesce.cpp       | 22 ++++++++++++++++++++++
>   5 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 6420e4d..8c4c08a 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1172,7 +1172,7 @@ vec4_visitor::dump_instructions()
>   }
>
>   int
> -vec4_visitor::setup_attributes(int payload_reg)
> +vec4_vs_visitor::setup_attributes(int payload_reg)
>   {
>      int nr_attributes;
>      int attribute_map[VERT_ATTRIB_MAX + 1];
> @@ -1392,7 +1392,7 @@ vec4_visitor::run()
>      if (INTEL_DEBUG & DEBUG_SHADER_TIME)
>         emit_shader_time_begin();
>
> -   emit_attribute_fixups();
> +   emit_prolog();
>
>      /* Generate VS IR for main().  (the visitor only descends into
>       * functions called "main").
> @@ -1400,14 +1400,14 @@ vec4_visitor::run()
>      if (shader) {
>         visit_instructions(shader->ir);
>      } else {
> -      emit_vertex_program_code();
> +      emit_program_code();

I'm not against renaming emit_vertex_program_code(), but it doesn't seem 
strictly necessary since geometry assembly programs don't exist AFAIK.

Maybe abstract this a little higher up and have a virtual emit_code() 
function that is:
- GS: visit_instructions(shader->ir)
- VS: if (shader) visit_instructions(..) else emit_vertex_program_code()

Then emit_vertex_program_code() could just live entirely inside the 
vec4_vs_visitor class.

>      }
>      base_ir = NULL;
>
>      if (c->key.base.userclip_active && !c->key.base.uses_clip_distance)
>         setup_uniform_clipplane_values();
>
> -   emit_urb_writes();
> +   emit_thread_end();
>
>      /* Before any optimization, push array accesses out to scratch
>       * space where we need them to be.  This pass may allocate new
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 5dbe128..76a840a 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -319,7 +319,6 @@ public:
>      void setup_uniform_clipplane_values();
>      void setup_uniform_values(ir_variable *ir);
>      void setup_builtin_uniform_values(ir_variable *ir);
> -   int setup_attributes(int payload_reg);
>      int setup_uniforms(int payload_reg);
>      void setup_payload();
>      bool reg_allocate_trivial();
> @@ -403,8 +402,6 @@ public:
>      void visit_instructions(const exec_list *list);
>
>      void setup_vp_regs();
> -   void emit_attribute_fixups();
> -   void emit_vertex_program_code();
>      void emit_vp_sop(uint32_t condmod, dst_reg dst,
>                       src_reg src0, src_reg src1, src_reg one);
>      dst_reg get_vp_dst_reg(const prog_dst_register &dst);
> @@ -453,7 +450,6 @@ public:
>      void emit_clip_distances(struct brw_reg reg, int offset);
>      void emit_generic_urb_slot(dst_reg reg, int varying);
>      void emit_urb_slot(int mrf, int varying);
> -   void emit_urb_writes(void);
>
>      void emit_shader_time_begin();
>      void emit_shader_time_end();
> @@ -484,6 +480,12 @@ public:
>
>      void dump_instruction(vec4_instruction *inst);
>      void dump_instructions();
> +
> +protected:
> +   virtual int setup_attributes(int payload_reg) = 0;
> +   virtual void emit_prolog() = 0;
> +   virtual void emit_program_code() = 0;
> +   virtual void emit_thread_end() = 0;
>   };
>
>   class vec4_vs_visitor : public vec4_visitor
> @@ -495,6 +497,12 @@ public:
>                      struct gl_shader_program *prog,
>                      struct brw_shader *shader,
>                      void *mem_ctx);
> +
> +protected:
> +   virtual int setup_attributes(int payload_reg);
> +   virtual void emit_prolog();
> +   virtual void emit_program_code();
> +   virtual void emit_thread_end();
>   };
>
>   /**
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index f5f53ad..13a293a 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -902,7 +902,7 @@ with_writemask(dst_reg const & r, int mask)
>   }
>
>   void
> -vec4_visitor::emit_attribute_fixups()
> +vec4_vs_visitor::emit_prolog()
>   {
>      dst_reg sign_recovery_shift;
>      dst_reg normalize_factor;
> @@ -2602,7 +2602,7 @@ align_interleaved_urb_mlen(struct brw_context *brw, int mlen)
>    * The VUE layout is documented in Volume 2a.
>    */
>   void
> -vec4_visitor::emit_urb_writes()
> +vec4_vs_visitor::emit_thread_end()
>   {
>      /* MRF 0 is reserved for the debugger, so start with message header
>       * in MRF 1.
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_vp.cpp b/src/mesa/drivers/dri/i965/brw_vec4_vp.cpp
> index 13156dd..abb64ed 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_vp.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_vp.cpp
> @@ -65,7 +65,7 @@ reswizzle(src_reg orig, unsigned x, unsigned y, unsigned z, unsigned w)
>   }
>
>   void
> -vec4_visitor::emit_vertex_program_code()
> +vec4_vs_visitor::emit_program_code()
>   {
>      this->need_all_constants_in_pull_buffer = false;
>
> diff --git a/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp b/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp
> index 418edd2..60a993e 100644
> --- a/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp
> +++ b/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp
> @@ -52,6 +52,28 @@ public:
>         : vec4_visitor(brw, c, NULL, shader_prog, NULL, NULL)
>      {
>      }
> +
> +protected:
> +   virtual int setup_attributes(int payload_reg)
> +   {
> +      assert(!"Not reached");
> +      return 0;
> +   }
> +
> +   virtual void emit_prolog()
> +   {
> +      assert(!"Not reached");
> +   }
> +
> +   virtual void emit_program_code()
> +   {
> +      assert(!"Not reached");
> +   }
> +
> +   virtual void emit_thread_end()
> +   {
> +      assert(!"Not reached");
> +   }
>   };
>
>
>



More information about the mesa-dev mailing list