[Mesa-dev] [PATCH 13/78] i965/nir/vec4: Implement conditional statements (nir_cf_node_if)

[Iago Toral]

On Thu, 2015-07-02 at 10:11 -0700, Jason Ekstrand wrote:
> On Wed, Jul 1, 2015 at 11:44 PM, Iago Toral <itoral at igalia.com> wrote:
> > On Tue, 2015-06-30 at 09:30 -0700, Jason Ekstrand wrote:
> >> On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
> >> > From: Iago Toral Quiroga <itoral at igalia.com>
> >> >
> >> > The same we do in the FS NIR backend, only that here we need to consider
> >> > the number of components in the condition and adjust the swizzle
> >> > accordingly.
> >> >
> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
> >> > ---
> >> >  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 23 ++++++++++++++++++++++-
> >> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> >> > index 1ec75ee..d81b6a7 100644
> >> > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> >> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp
> >> > @@ -314,7 +314,28 @@ vec4_visitor::nir_emit_cf_list(exec_list *list)
> >> >  void
> >> >  vec4_visitor::nir_emit_if(nir_if *if_stmt)
> >> >  {
> >> > -   /* @TODO: Not yet implemented */
> >> > +   /* First, put the condition in f0 */
> >> > +   src_reg condition = get_nir_src(if_stmt->condition, BRW_REGISTER_TYPE_D);
> >> > +
> >> > +   int num_components = if_stmt->condition.is_ssa ?
> >> > +      if_stmt->condition.ssa->num_components :
> >> > +      if_stmt->condition.reg.reg->num_components;
> >> > +
> >> > +   condition.swizzle = brw_swizzle_for_size(num_components);
> >> > +
> >> > +   vec4_instruction *inst = emit(MOV(dst_null_d(), condition));
> >> > +   inst->conditional_mod = BRW_CONDITIONAL_NZ;
> >>
> >> NIR if statements read only one component by definition.  There's no
> >> need to do this.
> >
> > I see, we still need to do this explicitly though:
> >
> > condition.swizzle = brw_swizzle_for_size(1);
> >
> > Maybe we should just make get_nir_src() set the swizzle based on the
> > number of components instead so we don't have to do this kind of things
> > after calling that, does that sound better?
> 
> Just pass the number of components into get_nir_src()?  That sounds fine to me.
> --Jason

Is that better? In this case it is sufficient because it is always 1,
but in other cases where we call get_nir_src I imagine we would also
want to get a register with the swizzle preset based on the number of
components the nir src, right? In that case, the number of components we
would need to pass would come from doing something like:

int num_components = src.is_ssa ?
                        src.ssa->num_components :
                        src.reg.reg->num_components;

and doing this every time we call get_nir_src does not look very nice to
me. Since we are already passing the nir src to get_nir_src, wouldn't it
be better if we had that function do this for us?

Iago

> >>
> >> > +   emit(IF(BRW_PREDICATE_NORMAL));
> >> > +
> >> > +   nir_emit_cf_list(&if_stmt->then_list);
> >> > +
> >> > +   /* note: if the else is empty, dead CF elimination will remove it */
> >> > +   emit(BRW_OPCODE_ELSE);
> >> > +
> >> > +   nir_emit_cf_list(&if_stmt->else_list);
> >> > +
> >> > +   emit(BRW_OPCODE_ENDIF);
> >> >  }
> >> >
> >> >  void
> >> > --
> >> > 2.1.4
> >> >
> >> > _______________________________________________
> >> > mesa-dev mailing list
> >> > mesa-dev at lists.freedesktop.org
> >> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
>