Re: [Mesa-dev] [PATCH 20/21] intel/compiler: Allocate pull_param in assign_constant_locations

[Jason Ekstrand]

On Fri, Oct 6, 2017 at 9:40 AM, Jordan Justen 
wrote:

> On 2017-09-29 14:25:20, Jason Ekstrand wrote:
> > diff --git a/src/intel/compiler/brw_vec4_visitor.cpp
> b/src/intel/compiler/brw_vec4_visitor.cpp
> > index ff5cd2d..ae51619 100644
> > --- a/src/intel/compiler/brw_vec4_visitor.cpp
> > +++ b/src/intel/compiler/brw_vec4_visitor.cpp
> > @@ -1782,6 +1782,11 @@ vec4_visitor::move_uniform_array_access_to_pull_
> constants()
> >return;
> > }
> >
> > +   /* Allocate the pull_params array */
> > +   assert(stage_prog_data->nr_pull_params == 0);
> > +   stage_prog_data->pull_param = ralloc_array(mem_ctx, uint32_t,
> > +  this->uniforms * 4);
>
> Why allocate this based on this->uniforms if nr_pull_params is 0?
>

At this point in the function nr_pull_params is always 0.  It gets assigned
later on in the function but we know that it will be <= uniforms * 4.

--Jason
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 20/21] intel/compiler: Allocate pull_param in assign_constant_locations

[Jordan Justen]

On 2017-09-29 14:25:20, Jason Ekstrand wrote:
> diff --git a/src/intel/compiler/brw_vec4_visitor.cpp 
> b/src/intel/compiler/brw_vec4_visitor.cpp
> index ff5cd2d..ae51619 100644
> --- a/src/intel/compiler/brw_vec4_visitor.cpp
> +++ b/src/intel/compiler/brw_vec4_visitor.cpp
> @@ -1782,6 +1782,11 @@ 
> vec4_visitor::move_uniform_array_access_to_pull_constants()
>return;
> }
>  
> +   /* Allocate the pull_params array */
> +   assert(stage_prog_data->nr_pull_params == 0);
> +   stage_prog_data->pull_param = ralloc_array(mem_ctx, uint32_t,
> +  this->uniforms * 4);

Why allocate this based on this->uniforms if nr_pull_params is 0?

-Jordan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 20/21] intel/compiler: Allocate pull_param in assign_constant_locations

[Jason Ekstrand]

Now that everything is nicely ralloc'd, we can allocate the pull_param
array in assign_constant_locations instead of higher up.  We can also
re-allocate the param array so that it's exactly the needed size.  This
should save us some memory because we're not allocating the total needed
param space for both push and pull.
---
 src/intel/compiler/brw_fs.cpp  | 15 +--
 src/intel/compiler/brw_vec4_visitor.cpp|  5 +
 src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp |  2 --
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 52b6628..b06ae9f 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -2085,14 +2085,17 @@ fs_visitor::assign_constant_locations()
if (thread_local_id_index >= 0)
   push_constant_loc[thread_local_id_index] = num_push_constants++;
 
-   /* As the uniforms are going to be reordered, take the data from a temporary
-* copy of the original param[].
+   /* As the uniforms are going to be reordered, stash the old array and
+* create two new arrays for push/pull params.
 */
-   uint32_t *param = ralloc_array(NULL, uint32_t, stage_prog_data->nr_params);
-   memcpy(param, stage_prog_data->param,
-  sizeof(uint32_t) * stage_prog_data->nr_params);
+   uint32_t *param = stage_prog_data->param;
stage_prog_data->nr_params = num_push_constants;
-   stage_prog_data->nr_pull_params = num_pull_constants;
+   stage_prog_data->param = ralloc_array(NULL, uint32_t, num_push_constants);
+   if (num_pull_constants > 0) {
+  stage_prog_data->nr_pull_params = num_pull_constants;
+  stage_prog_data->pull_param = ralloc_array(NULL, uint32_t,
+ num_pull_constants);
+   }
 
/* Now that we know how many regular uniforms we'll push, reduce the
 * UBO push ranges so we don't exceed the 3DSTATE_CONSTANT limits.
diff --git a/src/intel/compiler/brw_vec4_visitor.cpp 
b/src/intel/compiler/brw_vec4_visitor.cpp
index ff5cd2d..ae51619 100644
--- a/src/intel/compiler/brw_vec4_visitor.cpp
+++ b/src/intel/compiler/brw_vec4_visitor.cpp
@@ -1782,6 +1782,11 @@ 
vec4_visitor::move_uniform_array_access_to_pull_constants()
   return;
}
 
+   /* Allocate the pull_params array */
+   assert(stage_prog_data->nr_pull_params == 0);
+   stage_prog_data->pull_param = ralloc_array(mem_ctx, uint32_t,
+  this->uniforms * 4);
+
int pull_constant_loc[this->uniforms];
memset(pull_constant_loc, -1, sizeof(pull_constant_loc));
 
diff --git a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp 
b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
index a3e7b12..62755fd 100644
--- a/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
+++ b/src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp
@@ -195,7 +195,6 @@ brw_nir_setup_glsl_uniforms(void *mem_ctx, nir_shader 
*shader,
unsigned nr_params = shader->num_uniforms / 4;
stage_prog_data->nr_params = nr_params;
stage_prog_data->param = rzalloc_array(mem_ctx, uint32_t, nr_params);
-   stage_prog_data->pull_param = rzalloc_array(mem_ctx, uint32_t, nr_params);
 
nir_foreach_variable(var, >uniforms) {
   /* UBO's, atomics and samplers don't take up space in the
@@ -223,7 +222,6 @@ brw_nir_setup_arb_uniforms(void *mem_ctx, nir_shader 
*shader,
unsigned nr_params = plist->NumParameters * 4;
stage_prog_data->nr_params = nr_params;
stage_prog_data->param = rzalloc_array(mem_ctx, uint32_t, nr_params);
-   stage_prog_data->pull_param = rzalloc_array(mem_ctx, uint32_t, nr_params);
 
/* For ARB programs, prog_to_nir generates a single "parameters" variable
 * for all uniform data.  nir_lower_wpos_ytransform may also create an
-- 
2.5.0.400.gff86faf

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev