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

[Pohjolainen, Topi]

On Mon, May 09, 2016 at 07:33:02AM +0200, 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.

Nice, thanks. You can also add:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>