On Mon, Aug 24, 2015 at 4:48 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> 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*)
Sure. I can throw one in.
>> + * 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();
>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
