[Mesa-dev] [PATCH] nir: fix nir_before/after_cf_node() for function impls

[Rob Clark]

On Thu, May 5, 2016 at 2:11 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> On Thu, May 5, 2016 at 1:20 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> On Thu, May 5, 2016 at 10:10 AM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>>>
>>> Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
>>> ---
>>>  src/compiler/nir/nir.h | 22 ++++++++++++++++++----
>>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>>> index 98451c6..8316b0d 100644
>>> --- a/src/compiler/nir/nir.h
>>> +++ b/src/compiler/nir/nir.h
>>> @@ -1995,19 +1995,33 @@ nir_after_block_before_jump(nir_block *block)
>>>  static inline nir_cursor
>>>  nir_before_cf_node(nir_cf_node *node)
>>>  {
>>> -   if (node->type == nir_cf_node_block)
>>> +   switch(node->type) {
>>> +   case nir_cf_node_block:
>>>        return nir_before_block(nir_cf_node_as_block(node));
>>>
>>> -   return nir_after_block(nir_cf_node_as_block(nir_cf_node_prev(node)));
>>> +   case nir_cf_node_function:
>>> +      return nir_before_block(
>>> +         nir_start_block(nir_cf_node_as_function(node)));
>>
>>
>> I'm not sure what I think about this.  Every other nir_before_cf_node case
>> that touches a "container" node puts it *before* the container not at the
>> top of it.  This is a substantial edge-case.  What's the use-case?
>
> That's a good point. On the other hand, this is kind of the only sane
> thing that this can do. It was one of Rob's passes that was
> segfaulting. Is there a branch somewhere with the code that was doing
> this?

fwiw, just stick:
      nir_before_cf_node(&nir_shader_get_entrypoint(nir)->impl->cf_node);

after the glsl_to_nir() call in bdw.. that is same thing as what the
lowering pass was doing.

Jason suggested to use nir_before_cf_list(&impl->body) instead, which
probably should work..  although kinda nasty that nir_before_cf_node()
doesn't work for this.  (And I guess we should either fix it or
document it as a special case.)

BR,
-R

>>
>>>
>>> +
>>> +   default:
>>> +      return
>>> nir_after_block(nir_cf_node_as_block(nir_cf_node_prev(node)));
>>> +   }
>>>  }
>>>
>>>  static inline nir_cursor
>>>  nir_after_cf_node(nir_cf_node *node)
>>>  {
>>> -   if (node->type == nir_cf_node_block)
>>> +   switch(node->type) {
>>> +   case nir_cf_node_block:
>>>        return nir_after_block(nir_cf_node_as_block(node));
>>>
>>> -   return nir_before_block(nir_cf_node_as_block(nir_cf_node_next(node)));
>>> +   case nir_cf_node_function:
>>> +      return nir_after_block(
>>> +         nir_impl_last_block(nir_cf_node_as_function(node)));
>>> +
>>> +   default:
>>> +      return
>>> nir_before_block(nir_cf_node_as_block(nir_cf_node_next(node)));
>>> +   }
>>>  }
>>>
>>>  static inline nir_cursor
>>> --
>>> 2.5.5
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>