[Mesa-dev] [PATCH v2 56/59] i965/fs: push first double-based uniforms in push constant buffer

[Kenneth Graunke]

On Monday, May 9, 2016 7:33:02 AM PDT Samuel Iglesias Gonsálvez wrote:
> 
> On 09/05/16 07:21, Pohjolainen, Topi wrote:
> > On Fri, May 06, 2016 at 08:56:08AM +0200, Samuel Iglesias Gons?lvez wrote:
> >> When there is a mix of definitions of uniforms with 32-bit or 64-bit
> >> data type sizes, the driver ends up doing misaligned access to double
> >> based variables in the push constant buffer.
> >>
> >> To fix this, this patch pushes first all the 64-bit variables and
> >> then the rest. Then, all the variables would be aligned to
> >> its data type size.
> >>
> >> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> >> ---
> >>  src/mesa/drivers/dri/i965/brw_fs.cpp | 113 ++++++++++++++++++++++++
+----------
> >>  1 file changed, 83 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/
i965/brw_fs.cpp
> >> index 65aa3c7..7eaf1b4 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> @@ -39,6 +39,7 @@
> >>  #include "brw_program.h"
> >>  #include "brw_dead_control_flow.h"
> >>  #include "compiler/glsl_types.h"
> >> +#include "program/prog_parameter.h"
> >>  
> >>  using namespace brw;
> >>  
> >> @@ -2004,6 +2005,45 @@ fs_visitor::compact_virtual_grfs()
> >>     return progress;
> >>  }
> >>  
> >> +static void
> >> +set_push_pull_constant_loc(unsigned uniform, int &chunk_start, bool 
contiguous,
> >> +                           int *push_constant_loc, int 
*pull_constant_loc,
> >> +                           unsigned &num_push_constants,
> >> +                           unsigned &num_pull_constants,
> > 
> > I don't remember us using writable references elsewhere, and vaguely 
recall
> > there being a debate about this and decision to use pointers instead?
> > 
> 
> OK, I will change it to use pointers instead. I had no idea this was
> discussed before.

Oh, good catch.  We usually avoid pass-by-reference unless it's const,
and instead return out parameters via pointers.

Paul Berry, Chris Forbes, and I discussed this briefly in August 2013.
I'd suggested passing by reference, and both Paul and Chris preferred
pointers.  I believe a number of other developers expressed a preference
for pointers on IRC / in-person as well.

https://lists.freedesktop.org/archives/mesa-dev/2013-August/043593.html

I certainly wouldn't expect you to have seen that obscure discussion :)

--Ken
-------------- 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: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160508/6bd5b6d0/attachment.sig>