On Wednesday, August 19, 2015 10:45:48 PM Jason Ekstrand wrote: > The comment above move_uniform_array_access_to_pull_constants was > completely bogus because it has nothing to do with lowering instructions. > Instead, it's assiging locations of pull constants. > --- > src/mesa/drivers/dri/i965/brw_fs.cpp | 38 > +++++++++--------------------------- > src/mesa/drivers/dri/i965/brw_fs.h | 1 - > 2 files changed, 9 insertions(+), 30 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp > b/src/mesa/drivers/dri/i965/brw_fs.cpp > index 68bcbd0..b4003e0 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp > @@ -1766,21 +1766,19 @@ fs_visitor::compact_virtual_grfs() > return progress; > } > > -/* > - * Implements array access of uniforms by inserting a > - * PULL_CONSTANT_LOAD instruction. > +/** > + * Assign UNIFORM file registers to either push constants or pull constants. > * > - * Unlike temporary GRF array access (where we don't support it due to > - * the difficulty of doing relative addressing on instruction > - * destinations), we could potentially do array access of uniforms > - * that were loaded in GRF space as push constants. In real-world > - * usage we've seen, though, the arrays being used are always larger > - * than we could load as push constants, so just always move all > - * uniform array access out to a pull constant buffer.
It might make sense to leave a comment (for now) saying that we demote
all indirect addressing of uniform arrays to pull constants.
(Admittedly, that comment would be removed at the end of your patch
series, but depending how much we merge now vs. later...*shrug*)
> + * We allow a fragment shader to have more than the specified minimum
> + * maximum number of fragment shader uniform components (64). If
> + * there are too many of these, they'd fill up all of register space.
> + * So, this will push some of them out to the pull constant buffer and
> + * update the program to load them.
> */
> void
> -fs_visitor::move_uniform_array_access_to_pull_constants()
> +fs_visitor::assign_constant_locations()
> {
> + /* Only the first compile (SIMD8 mode) gets to decide on locations. */
> if (dispatch_width != 8)
> return;
>
> @@ -1817,23 +1815,6 @@
> fs_visitor::move_uniform_array_access_to_pull_constants()
> }
> }
> }
> -}
> -
> -/**
> - * Assign UNIFORM file registers to either push constants or pull constants.
> - *
> - * We allow a fragment shader to have more than the specified minimum
> - * maximum number of fragment shader uniform components (64). If
> - * there are too many of these, they'd fill up all of register space.
> - * So, this will push some of them out to the pull constant buffer and
> - * update the program to load them.
> - */
> -void
> -fs_visitor::assign_constant_locations()
> -{
> - /* Only the first compile (SIMD8 mode) gets to decide on locations. */
> - if (dispatch_width != 8)
> - return;
>
> /* Find which UNIFORM registers are still in use. */
> bool is_live[uniforms];
> @@ -4805,7 +4786,6 @@ fs_visitor::optimize()
>
> split_virtual_grfs();
>
> - move_uniform_array_access_to_pull_constants();
> assign_constant_locations();
> demote_pull_constants();
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index 6bca762..31f39fe 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -146,7 +146,6 @@ public:
> void spill_reg(int spill_reg);
> void split_virtual_grfs();
> bool compact_virtual_grfs();
> - void move_uniform_array_access_to_pull_constants();
> void assign_constant_locations();
> void demote_pull_constants();
> void invalidate_live_intervals();
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
