nir: do not pack varying with different types

Submitted by Samuel Pitoiset on April 3, 2019, 11:37 a.m.

Details

Message ID 20190403113729.31167-1-samuel.pitoiset@gmail.com
State Accepted
Headers show
Series "nir: do not pack varying with different types" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Samuel Pitoiset April 3, 2019, 11:37 a.m.
The current algorithm only supports packing 32-bit types.
If a shader uses both 16-bit and 32-bit varyings, we shouldn't
compact them together.

Cc: Timothy Arceri <tarceri@itsqueeze.com>
Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
---
 src/compiler/nir/nir_linking_helpers.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/compiler/nir/nir_linking_helpers.c b/src/compiler/nir/nir_linking_helpers.c
index 146d4e4e591..482ac2881bc 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -243,6 +243,7 @@  struct assigned_comps
    uint8_t comps;
    uint8_t interp_type;
    uint8_t interp_loc;
+   uint8_t is_32bit;
 };
 
 /* Packing arrays and dual slot varyings is difficult so to avoid complex
@@ -308,6 +309,7 @@  get_unmoveable_components_masks(struct exec_list *var_list,
             comps[location + i].interp_type =
                get_interp_type(var, type, default_to_smooth_interp);
             comps[location + i].interp_loc = get_interp_loc(var);
+            comps[location + i].is_32bit = glsl_type_is_32bit(glsl_without_array(type));
          }
       }
    }
@@ -572,6 +574,14 @@  assign_remap_locations(struct varying_loc (*remap)[4],
             continue;
          }
 
+         /* We can only pack varyings with matching types, and the current
+          * algorithm only supports packing 32-bit.
+          */
+         if (!assigned_comps[tmp_cursor].is_32bit) {
+            tmp_comp = 0;
+            continue;
+         }
+
          while (tmp_comp < 4 &&
                 (assigned_comps[tmp_cursor].comps & (1 << tmp_comp))) {
             tmp_comp++;

Comments

Seems ok for now.

Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>

On 3/4/19 10:37 pm, Samuel Pitoiset wrote:
> The current algorithm only supports packing 32-bit types.
> If a shader uses both 16-bit and 32-bit varyings, we shouldn't
> compact them together.
> 
> Cc: Timothy Arceri <tarceri@itsqueeze.com>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>
> ---
>   src/compiler/nir/nir_linking_helpers.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/src/compiler/nir/nir_linking_helpers.c b/src/compiler/nir/nir_linking_helpers.c
> index 146d4e4e591..482ac2881bc 100644
> --- a/src/compiler/nir/nir_linking_helpers.c
> +++ b/src/compiler/nir/nir_linking_helpers.c
> @@ -243,6 +243,7 @@ struct assigned_comps
>      uint8_t comps;
>      uint8_t interp_type;
>      uint8_t interp_loc;
> +   uint8_t is_32bit;
>   };
>   
>   /* Packing arrays and dual slot varyings is difficult so to avoid complex
> @@ -308,6 +309,7 @@ get_unmoveable_components_masks(struct exec_list *var_list,
>               comps[location + i].interp_type =
>                  get_interp_type(var, type, default_to_smooth_interp);
>               comps[location + i].interp_loc = get_interp_loc(var);
> +            comps[location + i].is_32bit = glsl_type_is_32bit(glsl_without_array(type));
>            }
>         }
>      }
> @@ -572,6 +574,14 @@ assign_remap_locations(struct varying_loc (*remap)[4],
>               continue;
>            }
>   
> +         /* We can only pack varyings with matching types, and the current
> +          * algorithm only supports packing 32-bit.
> +          */
> +         if (!assigned_comps[tmp_cursor].is_32bit) {
> +            tmp_comp = 0;
> +            continue;
> +         }
> +
>            while (tmp_comp < 4 &&
>                   (assigned_comps[tmp_cursor].comps & (1 << tmp_comp))) {
>               tmp_comp++;
>