Re: [Mesa-dev] [PATCH 2/2] nir: don't pack varyings ints with floats unless flat

2018-11-11 Thread Jason Ekstrand 
On Sun, Nov 11, 2018 at 9:37 PM Timothy Arceri 
wrote:

> On 12/11/18 2:11 pm, Jason Ekstrand wrote:
> > On November 11, 2018 20:30:29 Timothy Arceri 
> wrote:
> >
> >> Fixes: 1c9c42d16b4c ("nir: add varying component packing helpers")
> >> ---
> >> src/compiler/nir/nir_linking_helpers.c | 9 ++---
> >> 1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >>
> >> diff --git a/src/compiler/nir/nir_linking_helpers.c
> >> b/src/compiler/nir/nir_linking_helpers.c
> >> index b07bb40894e..125a8d55b3e 100644
> >> --- a/src/compiler/nir/nir_linking_helpers.c
> >> +++ b/src/compiler/nir/nir_linking_helpers.c
> >> @@ -196,10 +196,13 @@ nir_remove_unused_varyings(nir_shader *producer,
> >> nir_shader *consumer)
> >> }
> >>
> >> static uint8_t
> >> -get_interp_type(nir_variable *var, bool default_to_smooth_interp)
> >> +get_interp_type(nir_variable *var, const struct glsl_type *type,
> >> +bool default_to_smooth_interp)
> >> {
> >>if (var->data.interpolation != INTERP_MODE_NONE)
> >>   return var->data.interpolation;
> >> +   else if (glsl_type_is_integer(type))
> >> +  return INTERP_MODE_FLAT;
> >
> > Are we guaranteed that there will be no explititly interpolated integers?
>
> The GLSL 1.30 spec used to require ints to be marked as flat but the
> 4.60 spec doesn't say much at all. I can make "if
> (glsl_type_is_integer(type))" the first check, this would match what we
> do in GLSL IR. Would this cover your concerns?
>

Yes, it would.  With that, both are

Reviewed-by: Jason Ekstrand 


> >
> >>else if (default_to_smooth_interp)
> >>   return INTERP_MODE_SMOOTH;
> >>else
> >> @@ -253,7 +256,7 @@ get_slot_component_masks_and_interp_types(struct
> >> exec_list *var_list,
> >>  unsigned comps_slot2 = 0;
> >>  for (unsigned i = 0; i < slots; i++) {
> >> interp_type[location + i] =
> >> -   get_interp_type(var, default_to_smooth_interp);
> >> +   get_interp_type(var, type, default_to_smooth_interp);
> >> interp_loc[location + i] = get_interp_loc(var);
> >>
> >> if (dual_slot) {
> >> @@ -425,7 +428,7 @@ compact_components(nir_shader *producer,
> >> nir_shader *consumer, uint8_t *comps,
> >> continue;
> >>
> >>  bool found_new_offset = false;
> >> - uint8_t interp = get_interp_type(var,
> >> default_to_smooth_interp);
> >> + uint8_t interp = get_interp_type(var, type,
> >> default_to_smooth_interp);
> >>  for (; cursor[interp] < 32; cursor[interp]++) {
> >> uint8_t cursor_used_comps = comps[cursor[interp]];
> >>
> >> --
> >> 2.19.1
> >>
> >>
> >> ___
> >> mesa-dev mailing list
> >> mesa-dev@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] nir: don't pack varyings ints with floats unless flat

2018-11-11 Thread Timothy Arceri 

On 12/11/18 2:11 pm, Jason Ekstrand wrote:

On November 11, 2018 20:30:29 Timothy Arceri  wrote:


Fixes: 1c9c42d16b4c ("nir: add varying component packing helpers")
---
src/compiler/nir/nir_linking_helpers.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)


diff --git a/src/compiler/nir/nir_linking_helpers.c 
b/src/compiler/nir/nir_linking_helpers.c

index b07bb40894e..125a8d55b3e 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -196,10 +196,13 @@ nir_remove_unused_varyings(nir_shader *producer, 
nir_shader *consumer)

}

static uint8_t
-get_interp_type(nir_variable *var, bool default_to_smooth_interp)
+get_interp_type(nir_variable *var, const struct glsl_type *type,
+    bool default_to_smooth_interp)
{
   if (var->data.interpolation != INTERP_MODE_NONE)
  return var->data.interpolation;
+   else if (glsl_type_is_integer(type))
+  return INTERP_MODE_FLAT;


Are we guaranteed that there will be no explititly interpolated integers?


The GLSL 1.30 spec used to require ints to be marked as flat but the 
4.60 spec doesn't say much at all. I can make "if 
(glsl_type_is_integer(type))" the first check, this would match what we 
do in GLSL IR. Would this cover your concerns?






   else if (default_to_smooth_interp)
  return INTERP_MODE_SMOOTH;
   else
@@ -253,7 +256,7 @@ get_slot_component_masks_and_interp_types(struct 
exec_list *var_list,

 unsigned comps_slot2 = 0;
 for (unsigned i = 0; i < slots; i++) {
    interp_type[location + i] =
-   get_interp_type(var, default_to_smooth_interp);
+   get_interp_type(var, type, default_to_smooth_interp);
    interp_loc[location + i] = get_interp_loc(var);

    if (dual_slot) {
@@ -425,7 +428,7 @@ compact_components(nir_shader *producer, 
nir_shader *consumer, uint8_t *comps,

    continue;

 bool found_new_offset = false;
- uint8_t interp = get_interp_type(var, 
default_to_smooth_interp);
+ uint8_t interp = get_interp_type(var, type, 
default_to_smooth_interp);

 for (; cursor[interp] < 32; cursor[interp]++) {
    uint8_t cursor_used_comps = comps[cursor[interp]];

--
2.19.1


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





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

Re: [Mesa-dev] [PATCH 2/2] nir: don't pack varyings ints with floats unless flat

2018-11-11 Thread Jason Ekstrand 

On November 11, 2018 20:30:29 Timothy Arceri  wrote:


Fixes: 1c9c42d16b4c ("nir: add varying component packing helpers")
---
src/compiler/nir/nir_linking_helpers.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)


diff --git a/src/compiler/nir/nir_linking_helpers.c 
b/src/compiler/nir/nir_linking_helpers.c

index b07bb40894e..125a8d55b3e 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -196,10 +196,13 @@ nir_remove_unused_varyings(nir_shader *producer, 
nir_shader *consumer)

}

static uint8_t
-get_interp_type(nir_variable *var, bool default_to_smooth_interp)
+get_interp_type(nir_variable *var, const struct glsl_type *type,
+bool default_to_smooth_interp)
{
   if (var->data.interpolation != INTERP_MODE_NONE)
  return var->data.interpolation;
+   else if (glsl_type_is_integer(type))
+  return INTERP_MODE_FLAT;


Are we guaranteed that there will be no explititly interpolated integers?


   else if (default_to_smooth_interp)
  return INTERP_MODE_SMOOTH;
   else
@@ -253,7 +256,7 @@ get_slot_component_masks_and_interp_types(struct 
exec_list *var_list,

 unsigned comps_slot2 = 0;
 for (unsigned i = 0; i < slots; i++) {
interp_type[location + i] =
-   get_interp_type(var, default_to_smooth_interp);
+   get_interp_type(var, type, default_to_smooth_interp);
interp_loc[location + i] = get_interp_loc(var);

if (dual_slot) {
@@ -425,7 +428,7 @@ compact_components(nir_shader *producer, nir_shader 
*consumer, uint8_t *comps,

continue;

 bool found_new_offset = false;
- uint8_t interp = get_interp_type(var, default_to_smooth_interp);
+ uint8_t interp = get_interp_type(var, type, 
default_to_smooth_interp);

 for (; cursor[interp] < 32; cursor[interp]++) {
uint8_t cursor_used_comps = comps[cursor[interp]];

--
2.19.1


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




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

[Mesa-dev] [PATCH 2/2] nir: don't pack varyings ints with floats unless flat

2018-11-11 Thread Timothy Arceri 
Fixes: 1c9c42d16b4c ("nir: add varying component packing helpers")
---
 src/compiler/nir/nir_linking_helpers.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/compiler/nir/nir_linking_helpers.c 
b/src/compiler/nir/nir_linking_helpers.c
index b07bb40894e..125a8d55b3e 100644
--- a/src/compiler/nir/nir_linking_helpers.c
+++ b/src/compiler/nir/nir_linking_helpers.c
@@ -196,10 +196,13 @@ nir_remove_unused_varyings(nir_shader *producer, 
nir_shader *consumer)
 }
 
 static uint8_t
-get_interp_type(nir_variable *var, bool default_to_smooth_interp)
+get_interp_type(nir_variable *var, const struct glsl_type *type,
+bool default_to_smooth_interp)
 {
if (var->data.interpolation != INTERP_MODE_NONE)
   return var->data.interpolation;
+   else if (glsl_type_is_integer(type))
+  return INTERP_MODE_FLAT;
else if (default_to_smooth_interp)
   return INTERP_MODE_SMOOTH;
else
@@ -253,7 +256,7 @@ get_slot_component_masks_and_interp_types(struct exec_list 
*var_list,
  unsigned comps_slot2 = 0;
  for (unsigned i = 0; i < slots; i++) {
 interp_type[location + i] =
-   get_interp_type(var, default_to_smooth_interp);
+   get_interp_type(var, type, default_to_smooth_interp);
 interp_loc[location + i] = get_interp_loc(var);
 
 if (dual_slot) {
@@ -425,7 +428,7 @@ compact_components(nir_shader *producer, nir_shader 
*consumer, uint8_t *comps,
 continue;
 
  bool found_new_offset = false;
- uint8_t interp = get_interp_type(var, default_to_smooth_interp);
+ uint8_t interp = get_interp_type(var, type, default_to_smooth_interp);
  for (; cursor[interp] < 32; cursor[interp]++) {
 uint8_t cursor_used_comps = comps[cursor[interp]];
 
-- 
2.19.1

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