On Thu, Sep 17, 2015 at 1:09 AM, Eduardo Lima Mitev <el...@igalia.com> wrote:
> On 09/15/2015 10:44 PM, Jason Ekstrand wrote:
>> The idea here is not that it gives register coalescing a little bit of a
>> helping hand. It doesn't actually fix the coalescing problems, but it
>> seems to help a good bit.
>>
>> Shader-db results for vec4 programs on Haswell:
>>
>> total instructions in shared programs: 1746280 -> 1683959 (-3.57%)
>> instructions in affected programs: 1259166 -> 1196845 (-4.95%)
>> helped: 11363
>> HURT: 148
>>
>
> I get exactly the same numbers on my Haswell too. Nice!
>
> Some comments below.
>
>> v2 (Jason Ekstrand):
>> - Run nir_move_vec_src_uses_to_dest after going out of SSA
>> - New shader-db numbers
>> ---
>> src/glsl/nir/nir_move_vec_src_uses_to_dest.c | 16 ++++++++++++----
>> src/mesa/drivers/dri/i965/brw_nir.c | 2 ++
>> 2 files changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/glsl/nir/nir_move_vec_src_uses_to_dest.c
>> b/src/glsl/nir/nir_move_vec_src_uses_to_dest.c
>> index 6e248a2..4c9032d 100644
>> --- a/src/glsl/nir/nir_move_vec_src_uses_to_dest.c
>> +++ b/src/glsl/nir/nir_move_vec_src_uses_to_dest.c
>> @@ -79,17 +79,23 @@ move_vec_src_uses_to_dest_block(nir_block *block, void
>> *shader)
>> continue; /* The loop */
>> }
>>
>> + /* Can't handle non-SSA vec operations */
>> + if (!vec->dest.dest.is_ssa)
>> + continue;
>> +
>> /* Can't handle saturation */
>> if (vec->dest.saturate)
>> continue;
>>
>> - assert(vec->dest.dest.is_ssa);
>> -
>> /* First, mark all of the sources we are going to consider for
>> rewriting
>> * to the destination
>> */
>> int srcs_remaining = 0;
>> for (unsigned i = 0; i < nir_op_infos[vec->op].num_inputs; i++) {
>> + /* We can't rewrite a source if it's not in SSA form */
>> + if (!vec->src[i].src.is_ssa)
>> + continue;
>> +
>> /* We can't rewrite a source if it has modifiers */
>> if (vec->src[i].abs || vec->src[i].negate)
>> continue;
>> @@ -97,9 +103,11 @@ move_vec_src_uses_to_dest_block(nir_block *block, void
>> *shader)
>> srcs_remaining |= 1 << i;
>> }
>>
>> - for (unsigned i; i = ffs(srcs_remaining) - 1, srcs_remaining;) {
>> - assert(vec->src[i].src.is_ssa);
>> + /* We can't actually do anything with this instruction */
>> + if (srcs_remaining == 0)
>> + continue;
>>
>> + for (unsigned i; i = ffs(srcs_remaining) - 1, srcs_remaining;) {
>> int8_t swizzle[4] = { -1, -1, -1, -1 };
>>
>> for (unsigned j = i; j < nir_op_infos[vec->op].num_inputs; j++) {
>>
>
> I think all hunks above should be squashed into previous patch (3/4).
> With that, both patches 3/4 and this one 4/4 are:
Sorry about that, I failed at rebasing...
> Reviewed-by: Eduardo Lima Mitev <el...@igalia.com>
Thanks for all your reviewing!
--Jason
>> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c
>> b/src/mesa/drivers/dri/i965/brw_nir.c
>> index f326b23..8568988 100644
>> --- a/src/mesa/drivers/dri/i965/brw_nir.c
>> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
>> @@ -187,6 +187,8 @@ brw_create_nir(struct brw_context *brw,
>> nir_validate_shader(nir);
>>
>> if (!is_scalar) {
>> + nir_move_vec_src_uses_to_dest(nir);
>> + nir_validate_shader(nir);
>
> A blank line here maybe? Not that I care much.
Sure.
>> nir_lower_vec_to_movs(nir);
>> nir_validate_shader(nir);
>> }
>>
>
> Eduardo
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
