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:
Reviewed-by: Eduardo Lima Mitev <el...@igalia.com>
> 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.
> 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
