[Mesa-dev] [PATCH 08/30] i965/vec4: Virtualize setup_payload instead of setup_attributes.

[Paul Berry]

On 20 August 2013 11:30, Paul Berry <stereotype441 at gmail.com> wrote:

> When I initially generaized the vec4_visitor class in preparation for
> geometry shaders, I assumed that the setup_attributes() function would
> need to be different between vertex and geometry shaders, but its
> caller, setup_payload(), could be shared.  So I made
> setup_attributes() a virtual function.
>

>From our in-person code review yesterday:

s/generaized/generalized/ in the commit message.


>
> It turns out this isn't true; setup_payload() needs to be different
> too, since the geometry shader payload sometimes includes an extra
> register (primitive ID) that has to come before uniforms.
>
> So setup_payload() needs to be the virtual function instead of
> setup_attributes().
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp                    | 2 +-
>  src/mesa/drivers/dri/i965/brw_vec4.h                      | 6 +++---
>  src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp | 3 +--
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index bfef8e0..abdf3ab 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1288,7 +1288,7 @@ vec4_visitor::setup_uniforms(int reg)
>  }
>
>  void
> -vec4_visitor::setup_payload(void)
> +vec4_vs_visitor::setup_payload(void)
>  {
>     int reg = 0;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h
> b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 587cb45..171f14d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -367,7 +367,6 @@ public:
>     void setup_uniform_values(ir_variable *ir);
>     void setup_builtin_uniform_values(ir_variable *ir);
>     int setup_uniforms(int payload_reg);
> -   void setup_payload();
>     bool reg_allocate_trivial();
>     bool reg_allocate();
>     void evaluate_spill_costs(float *spill_costs, bool *no_spill);
> @@ -539,7 +538,7 @@ protected:
>     void emit_vertex();
>     void lower_attributes_to_hw_regs(const int *attribute_map);
>     virtual dst_reg *make_reg_for_system_value(ir_variable *ir) = 0;
> -   virtual int setup_attributes(int payload_reg) = 0;
> +   virtual void setup_payload() = 0;
>     virtual void emit_prolog() = 0;
>     virtual void emit_program_code() = 0;
>     virtual void emit_thread_end() = 0;
> @@ -562,7 +561,7 @@ public:
>
>  protected:
>     virtual dst_reg *make_reg_for_system_value(ir_variable *ir);
> -   virtual int setup_attributes(int payload_reg);
> +   virtual void setup_payload();
>     virtual void emit_prolog();
>     virtual void emit_program_code();
>     virtual void emit_thread_end();
> @@ -570,6 +569,7 @@ protected:
>     virtual vec4_instruction *emit_urb_write_opcode(bool complete);
>
>  private:
> +   int setup_attributes(int payload_reg);
>     void setup_vp_regs();
>     dst_reg get_vp_dst_reg(const prog_dst_register &dst);
>     src_reg get_vp_src_reg(const prog_src_register &src);
> 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 e141305..ab4498b 100644
> --- a/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp
> +++ b/src/mesa/drivers/dri/i965/test_vec4_register_coalesce.cpp
> @@ -60,10 +60,9 @@ protected:
>        return NULL;
>     }
>
> -   virtual int setup_attributes(int payload_reg)
> +   virtual void setup_payload()
>     {
>        assert(!"Not reached");
> -      return 0;
>     }
>
>     virtual void emit_prolog()
> --
> 1.8.3.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130822/e5cc78fe/attachment-0001.html>