[Mesa-dev] [PATCH] i965: Store uniform constant values in a gl_constant_value instead of float

[Kenneth Graunke]

On Tuesday, August 12, 2014 07:04:23 PM Neil Roberts wrote:
> Hi,
> 
> Here is a replacement patch for the one to use memcpy when copying
> uniform values into the batch buffer here:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2014-August/065179.html
> 
> Eric Anholt suggested instead of using memcpy we should change the
> type of the pointer to uint32_t. I started to implement that idea but
> I found there were quite a few bits of debugging code that are trying
> to print the values as floats. I thought it might be cleaner if we
> make the pointers gl_constant_value instead because that will avoid
> having ugly casts in this debugging code and also makes it more
> obvious that the values are being overloaded.
> 
> This patch also ends up fixing a few other potential problems in
> places where we are copying floatings for example when copying into
> the pull params.
> 
> I've run the patch through all of the Piglit tests on Ivybridge with a
> 32-bit build and it doesn't cause any changes except to fix the five
> test cases mentioned in the bug report.
> 
> - Neil
> 
> ------- >8 --------------- (use git am --scissors to automatically chop here)
> 
> The brw_stage_prog_data struct previously contained an array of float pointers
> to the values of parameters. These were then copied into a batch buffer to
> upload the values using a regular assignment. However the float values were
> also being overloaded to store integer values for integer uniforms. This can
> break if x87 floating-point registers are used to do the assignment because
> the fst instruction tries to fix up invalid float values. If an integer
> constant happened to look like an invalid float value then it would get
> altered when it was copied into the batch buffer.
> 
> This patch changes the pointers to be gl_constant_value instead so that the
> assignment should end up copying without any alteration. This also makes it
> more obvious that the values being stored here are overloaded for multiple
> types.
> 
> There are some static asserts where the values are uploaded to ensure that the
> size of gl_constant_value is the same as a float.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81150
> ---
>  src/mesa/drivers/dri/i965/brw_context.h          |  5 +++--
>  src/mesa/drivers/dri/i965/brw_curbe.c            | 22 ++++++++++++----------
>  src/mesa/drivers/dri/i965/brw_fs.cpp             |  6 +++---
>  src/mesa/drivers/dri/i965/brw_fs_fp.cpp          |  2 +-
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp     |  6 +++---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp           |  6 +++---
>  src/mesa/drivers/dri/i965/brw_vec4_gs.c          |  4 ++--
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   | 13 ++++++++-----
>  src/mesa/drivers/dri/i965/brw_vec4_vp.cpp        |  4 ++--
>  src/mesa/drivers/dri/i965/brw_vs.c               |  6 ++++--
>  src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 10 ++++++----
>  src/mesa/drivers/dri/i965/brw_wm.c               |  5 +++--
>  src/mesa/drivers/dri/i965/gen6_vs_state.c        |  8 +++++---
>  13 files changed, 55 insertions(+), 42 deletions(-)

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140812/221831da/attachment.sig>