Re: [Mesa-dev] [PATCH v3] nir: allow nir_lower_phis_to_scalar() on more src types

2019-02-22 Thread Jason Ekstrand
On Fri, Feb 22, 2019 at 5:21 PM Timothy Arceri 
wrote:

> Rather than only lowering if all srcs are scalarizable we instead
> check that at least one src is scalarizable.
>
> We remove undef type from the is_phi_src_scalarizable() as using
> this as a check just cause regressions when it is the only
> scalarizable src.
>
> total instructions in shared programs: 13219105 -> 13024547 (-1.47%)
> instructions in affected programs: 1153797 -> 959239 (-16.86%)
> helped: 581
> HURT: 74
>
> total cycles in shared programs: 333968972 -> 324807922 (-2.74%)
> cycles in affected programs: 129809402 -> 120648352 (-7.06%)
> helped: 571
> HURT: 131
>
> total spills in shared programs: 57947 -> 29130 (-49.73%)
> spills in affected programs: 53364 -> 24547 (-54.00%)
> helped: 351
> HURT: 0
>
> total fills in shared programs: 51310 -> 25468 (-50.36%)
> fills in affected programs: 44882 -> 19040 (-57.58%)
> helped: 351
> HURT: 0
> ---
>  src/compiler/nir/nir_lower_phis_to_scalar.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/compiler/nir/nir_lower_phis_to_scalar.c
> b/src/compiler/nir/nir_lower_phis_to_scalar.c
> index 16001f73685..b2ca89d3513 100644
> --- a/src/compiler/nir/nir_lower_phis_to_scalar.c
> +++ b/src/compiler/nir/nir_lower_phis_to_scalar.c
> @@ -75,7 +75,6 @@ is_phi_src_scalarizable(nir_phi_src *src,
>return should_lower_phi(nir_instr_as_phi(src_instr), state);
>
> case nir_instr_type_load_const:
> -   case nir_instr_type_ssa_undef:
>

Instead of removing it, would you mind handling it as

case nir_instr_type_ssa_undef:
/* The caller of this function is going to OR the results and we don't
want undefs to count so we return false. */
   return false;

With that,

Reviewed-by: Jason Ekstrand 


>/* These are trivially scalarizable */
>return true;
>
> @@ -150,11 +149,16 @@ should_lower_phi(nir_phi_instr *phi, struct
> lower_phis_to_scalar_state *state)
>  */
> entry = _mesa_hash_table_insert(state->phi_table, phi, (void
> *)(intptr_t)1);
>
> -   bool scalarizable = true;
> +   bool scalarizable = false;
>
> nir_foreach_phi_src(src, phi) {
> +  /* This loop ignores srcs that are not scalarizable because its
> likely
> +   * still worth copying to temps if another phi source is
> scalarizable.
> +   * This reduces register spilling by a huge amount in the i965
> driver for
> +   * Deus Ex: MD.
> +   */
>scalarizable = is_phi_src_scalarizable(src, state);
> -  if (!scalarizable)
> +  if (scalarizable)
>   break;
> }
>
> --
> 2.20.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v3] nir: allow nir_lower_phis_to_scalar() on more src types

2019-02-22 Thread Timothy Arceri
Rather than only lowering if all srcs are scalarizable we instead
check that at least one src is scalarizable.

We remove undef type from the is_phi_src_scalarizable() as using
this as a check just cause regressions when it is the only
scalarizable src.

total instructions in shared programs: 13219105 -> 13024547 (-1.47%)
instructions in affected programs: 1153797 -> 959239 (-16.86%)
helped: 581
HURT: 74

total cycles in shared programs: 333968972 -> 324807922 (-2.74%)
cycles in affected programs: 129809402 -> 120648352 (-7.06%)
helped: 571
HURT: 131

total spills in shared programs: 57947 -> 29130 (-49.73%)
spills in affected programs: 53364 -> 24547 (-54.00%)
helped: 351
HURT: 0

total fills in shared programs: 51310 -> 25468 (-50.36%)
fills in affected programs: 44882 -> 19040 (-57.58%)
helped: 351
HURT: 0
---
 src/compiler/nir/nir_lower_phis_to_scalar.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/compiler/nir/nir_lower_phis_to_scalar.c 
b/src/compiler/nir/nir_lower_phis_to_scalar.c
index 16001f73685..b2ca89d3513 100644
--- a/src/compiler/nir/nir_lower_phis_to_scalar.c
+++ b/src/compiler/nir/nir_lower_phis_to_scalar.c
@@ -75,7 +75,6 @@ is_phi_src_scalarizable(nir_phi_src *src,
   return should_lower_phi(nir_instr_as_phi(src_instr), state);
 
case nir_instr_type_load_const:
-   case nir_instr_type_ssa_undef:
   /* These are trivially scalarizable */
   return true;
 
@@ -150,11 +149,16 @@ should_lower_phi(nir_phi_instr *phi, struct 
lower_phis_to_scalar_state *state)
 */
entry = _mesa_hash_table_insert(state->phi_table, phi, (void *)(intptr_t)1);
 
-   bool scalarizable = true;
+   bool scalarizable = false;
 
nir_foreach_phi_src(src, phi) {
+  /* This loop ignores srcs that are not scalarizable because its likely
+   * still worth copying to temps if another phi source is scalarizable.
+   * This reduces register spilling by a huge amount in the i965 driver for
+   * Deus Ex: MD.
+   */
   scalarizable = is_phi_src_scalarizable(src, state);
-  if (!scalarizable)
+  if (scalarizable)
  break;
}
 
-- 
2.20.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev