<div dir="ltr">On 20 August 2013 11:30, Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">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. </blockquote><div> </div><div>>From our in-person code review yesterday: s/generaized/generalized/ in the commit message. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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 </blockquote></div> </div></div>