[Mesa-dev] [PATCH] nir: Use nir_shader_get_entrypoint in TCS quad workaround code.

[Connor Abbott]

On Thu, Aug 18, 2016 at 6:12 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Aug 18, 2016 23:02, "Connor Abbott" <cwabbott0 at gmail.com> wrote:
>>
>> On Thu, Aug 18, 2016 at 2:46 PM, Kenneth Graunke <kenneth at whitecape.org>
>> wrote:
>> > We want to insert the code at the end of the program.  Looping over
>> > all the functions (of which there was only one) was the old way of doing
>> > this, but now we have nir_shader_get_entrypoint(), so let's use it.
>> >
>> > Suggested by Connor Abbott.
>> >
>> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> > ---
>> >  .../drivers/dri/i965/brw_nir_tcs_workarounds.c     | 23
>> > +++++++++++-----------
>> >  1 file changed, 11 insertions(+), 12 deletions(-)
>> >
>> > Sorry, I pushed the patch rather quickly...here's a follow-on to use
>> > nir_shader_get_entrypoint().
>> >
>> > Shouldn't be also be using it in nir_lower_gs_intrinsics.c?
>> >
>> > diff --git a/src/mesa/drivers/dri/i965/brw_nir_tcs_workarounds.c
>> > b/src/mesa/drivers/dri/i965/brw_nir_tcs_workarounds.c
>> > index 0626981..ac4f9e0 100644
>> > --- a/src/mesa/drivers/dri/i965/brw_nir_tcs_workarounds.c
>> > +++ b/src/mesa/drivers/dri/i965/brw_nir_tcs_workarounds.c
>> > @@ -134,19 +134,18 @@ brw_nir_apply_tcs_quads_workaround(nir_shader
>> > *nir)
>> >  {
>> >     assert(nir->stage == MESA_SHADER_TESS_CTRL);
>> >
>> > -   nir_foreach_function(func, nir) {
>> > -      if (!func->impl)
>> > -         continue;
>> > +   nir_function *func = nir_shader_get_entrypoint(nir);
>> > +   if (!func->impl)
>> > +      return;
>>
>> Minor nit: I think you can drop the if, since it doesn't make sense to
>> have an entrypoint with only a declaration and no implementation.
>> Other than that,
>
> Mind if we split the difference and make it an assert?  We can probably add
> said assert too the get_entrypoint helper.

Sure, sounds good.

>
>> Reviewed-by: Connor Abbott <cwabbott0 at gmail.com>
>>
>> What about my other comment though?
>>
>> >
>> > -      nir_builder b;
>> > -      nir_builder_init(&b, func->impl);
>> > +   nir_builder b;
>> > +   nir_builder_init(&b, func->impl);
>> >
>> > -      struct set_entry *entry;
>> > -      set_foreach(func->impl->end_block->predecessors, entry) {
>> > -         nir_block *pred = (nir_block *) entry->key;
>> > -         emit_quads_workaround(&b, pred);
>> > -      }
>> > -
>> > -      nir_metadata_preserve(func->impl, 0);
>> > +   struct set_entry *entry;
>> > +   set_foreach(func->impl->end_block->predecessors, entry) {
>> > +      nir_block *pred = (nir_block *) entry->key;
>> > +      emit_quads_workaround(&b, pred);
>> >     }
>> > +
>> > +   nir_metadata_preserve(func->impl, 0);
>> >  }
>> > --
>> > 2.9.3
>> >
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev