Re: [Mesa-dev] [PATCH 1/3] nir: NULL check lower_copies_to_load_store()

2017-07-18 Thread Jason Ekstrand 
On Tue, Jul 18, 2017 at 4:26 PM, Timothy Arceri 
wrote:

> On 19/07/17 09:17, Jason Ekstrand wrote:
>
>> On Thu, Jun 29, 2017 at 7:45 PM, Timothy Arceri > > wrote:
>>
>> Allows us to disable array spliting for arrays of arrays without
>> regressing tests such as:
>>
>> ES31-CTS.functional.shaders.arrays_of_arrays.return.explicit
>> .struct_3x1x3_fragment
>> ---
>>   src/compiler/nir/nir_lower_vars_to_ssa.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/compiler/nir/nir_lower_vars_to_ssa.c
>> b/src/compiler/nir/nir_lower_vars_to_ssa.c
>> index e5a12eb..31f7e7a 100644
>> --- a/src/compiler/nir/nir_lower_vars_to_ssa.c
>> +++ b/src/compiler/nir/nir_lower_vars_to_ssa.c
>> @@ -441,7 +441,7 @@ static bool
>>   lower_copies_to_load_store(struct deref_node *node,
>>  struct lower_variables_state *state)
>>   {
>> -   if (!node->copies)
>> +   if (!node || !node->copies)
>>
>>
>> If we got a NULL node here, something is wrong.  I think this is just
>> papering over the issue.
>>
>
> It's possible this was only an issue before the fix in the following
> patch. I'll drop this and push to jenkins to see if this can be dropped.
>

It can't.  I tried.

I just sent a patch that fixes the iterator to never pass in NULL.  I'd
prefer we replace this patch with that one.  I've verified that it fixes
the test mentioned in the commit message.


>
>> return true;
>>
>>  struct set_entry *copy_entry;
>> --
>> 2.9.4
>>
>> ___
>> 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 1/3] nir: NULL check lower_copies_to_load_store()

2017-07-18 Thread Timothy Arceri 

On 19/07/17 09:17, Jason Ekstrand wrote:
On Thu, Jun 29, 2017 at 7:45 PM, Timothy Arceri > wrote:


Allows us to disable array spliting for arrays of arrays without
regressing tests such as:


ES31-CTS.functional.shaders.arrays_of_arrays.return.explicit.struct_3x1x3_fragment
---
  src/compiler/nir/nir_lower_vars_to_ssa.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/nir/nir_lower_vars_to_ssa.c
b/src/compiler/nir/nir_lower_vars_to_ssa.c
index e5a12eb..31f7e7a 100644
--- a/src/compiler/nir/nir_lower_vars_to_ssa.c
+++ b/src/compiler/nir/nir_lower_vars_to_ssa.c
@@ -441,7 +441,7 @@ static bool
  lower_copies_to_load_store(struct deref_node *node,
 struct lower_variables_state *state)
  {
-   if (!node->copies)
+   if (!node || !node->copies)


If we got a NULL node here, something is wrong.  I think this is just 
papering over the issue.


It's possible this was only an issue before the fix in the following 
patch. I'll drop this and push to jenkins to see if this can be dropped.




return true;

 struct set_entry *copy_entry;
--
2.9.4

___
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 1/3] nir: NULL check lower_copies_to_load_store()

2017-07-18 Thread Jason Ekstrand 
On Thu, Jun 29, 2017 at 7:45 PM, Timothy Arceri 
wrote:

> Allows us to disable array spliting for arrays of arrays without
> regressing tests such as:
>
> ES31-CTS.functional.shaders.arrays_of_arrays.return.
> explicit.struct_3x1x3_fragment
> ---
>  src/compiler/nir/nir_lower_vars_to_ssa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir_lower_vars_to_ssa.c
> b/src/compiler/nir/nir_lower_vars_to_ssa.c
> index e5a12eb..31f7e7a 100644
> --- a/src/compiler/nir/nir_lower_vars_to_ssa.c
> +++ b/src/compiler/nir/nir_lower_vars_to_ssa.c
> @@ -441,7 +441,7 @@ static bool
>  lower_copies_to_load_store(struct deref_node *node,
> struct lower_variables_state *state)
>  {
> -   if (!node->copies)
> +   if (!node || !node->copies)
>

If we got a NULL node here, something is wrong.  I think this is just
papering over the issue.


>return true;
>
> struct set_entry *copy_entry;
> --
> 2.9.4
>
> ___
> 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 1/3] nir: NULL check lower_copies_to_load_store()

2017-06-29 Thread Timothy Arceri 
Allows us to disable array spliting for arrays of arrays without
regressing tests such as:

ES31-CTS.functional.shaders.arrays_of_arrays.return.explicit.struct_3x1x3_fragment
---
 src/compiler/nir/nir_lower_vars_to_ssa.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/compiler/nir/nir_lower_vars_to_ssa.c 
b/src/compiler/nir/nir_lower_vars_to_ssa.c
index e5a12eb..31f7e7a 100644
--- a/src/compiler/nir/nir_lower_vars_to_ssa.c
+++ b/src/compiler/nir/nir_lower_vars_to_ssa.c
@@ -441,7 +441,7 @@ static bool
 lower_copies_to_load_store(struct deref_node *node,
struct lower_variables_state *state)
 {
-   if (!node->copies)
+   if (!node || !node->copies)
   return true;
 
struct set_entry *copy_entry;
-- 
2.9.4

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