On Thu, Mar 13, 2014 at 08:46:24PM -0700, Kenneth Graunke wrote:
> On 03/13/2014 05:55 PM, Pohjolainen, Topi wrote:
> > On Tue, Mar 11, 2014 at 11:48:51PM -0700, Kenneth Graunke wrote:
> >> Previously, both move_uniform_array_access_to_pull_constants() and
> >> setup_pull_constants() maintained stack-local arrays with this
> >> information. Storing this information will allow it to be used from
> >> multiple functions, allowing us to split and move code around.
> >>
> >> We'll also eventually want to pass pull constant location information
> >> to the SIMD16 compile. Saving this information will help us do that.
> >>
> >> Unfortunately, the two functions *cannot* share the contents of the
> >> array just yet. remove_dead_constants() renumbers all the UNIFORM
> >> registers to be contiguous starting at zero, so the two functions
> >> talk about uniforms using different names. We can't even remap them,
> >> since move_uniform_array_access_to_pull_constants() deletes UNIFORM
> >> registers that are only accessed with reladdr, so remove_dead_constants
> >> can't even see them.
> >>
> >> This situation will improve in the next few patches.
> >>
> >> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> >> ---
> >> src/mesa/drivers/dri/i965/brw_fs.cpp | 7 +++++--
> >> src/mesa/drivers/dri/i965/brw_fs.h | 6 ++++++
> >> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 1 +
> >> 3 files changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> index c24d2f8..8faf401 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> @@ -1827,7 +1827,7 @@ fs_visitor::remove_dead_constants()
> >> void
> >> fs_visitor::move_uniform_array_access_to_pull_constants()
> >> {
> >> - int pull_constant_loc[uniforms];
> >> + pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
> >>
> >> for (unsigned int i = 0; i < uniforms; i++) {
> >> pull_constant_loc[i] = -1;
> >> @@ -1884,6 +1884,9 @@
> >> fs_visitor::move_uniform_array_access_to_pull_constants()
> >> }
> >> }
> >> invalidate_live_intervals();
> >> +
> >> + ralloc_free(pull_constant_loc);
> >> + pull_constant_loc = NULL;
> >> }
> >>
> >> /**
> >> @@ -1909,7 +1912,7 @@ fs_visitor::setup_pull_constants()
> >> */
> >> unsigned int pull_uniform_base = max_uniform_components;
> >>
> >> - int pull_constant_loc[uniforms];
> >> + pull_constant_loc = ralloc_array(mem_ctx, int, uniforms);
> >> for (unsigned int i = 0; i < uniforms; i++) {
> >> if (i < pull_uniform_base) {
> >> pull_constant_loc[i] = -1;
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> >> b/src/mesa/drivers/dri/i965/brw_fs.h
> >> index 86c95df..abfdb10 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> >> @@ -509,6 +509,12 @@ public:
> >> /** Number of uniform variable components visited. */
> >> unsigned uniforms;
> >>
> >> + /**
> >> + * Array mapping UNIFORM register numbers to the pull parameter index,
> >> + * or -1 if this uniform register isn't being uploaded as a pull
> >> constant.
> >> + */
> >> + int *pull_constant_loc;
> >> +
> >> /* This is the map from UNIFORM hw_reg + reg_offset as generated by
> >> * the visitor to the packed uniform number after
> >> * remove_dead_constants() that represents the actual uploaded
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> >> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> >> index 90272eb..404af30 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> >> @@ -2971,6 +2971,7 @@ fs_visitor::fs_visitor(struct brw_context *brw,
> >> this->regs_live_at_ip = NULL;
> >>
> >> this->uniforms = 0;
> >> + this->pull_constant_loc = NULL;
> >
> > Apart from 'dispatch_width' all the other members are initialised here in
> > the
> > body of the constructor, so it is consistent to have this here also. I'm
> > just
> > thinking if we would like to start using more the initialization list.
>
> I don't have a strong preference. I feel like it's mostly a stylistic
> decision, and one I'm happy to leave up to the author.
Sounds fair, as far as I undertstand it is really only mandatory for constants
members.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
