On Sun, 2015-07-05 at 19:14 -0700, Jason Ekstrand wrote: > On Fri, Jul 3, 2015 at 12:32 AM, Iago Toral <ito...@igalia.com> wrote: > > On Thu, 2015-07-02 at 10:11 -0700, Jason Ekstrand wrote: > >> On Wed, Jul 1, 2015 at 11:44 PM, Iago Toral <ito...@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 <el...@igalia.com> > >> >> wrote: > >> >> > From: Iago Toral Quiroga <ito...@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; > > That's not quite what you want either. NIR doesn't guarantee that the > number of components used equals the number of components in the > register or ssa value. (We probably want to change that eventually). > However, you should always know how many components you have. For ALU > operations, it's either 4 (if it has a write-mask) or it's in the op > infos; for intrinsics it's either in the intrinsic_infos or in > intrin->num_components; for if's it's, always 1, etc. > > Does that make more sense?
Yes, thanks for explaining this. I recall now that Connor had mentioned something like this once as well. Iago > Also, this should affect the e-mail that I > just sent to Antia. > > --Jason > > > 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@lists.freedesktop.org > >> >> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev > >> >> _______________________________________________ > >> >> mesa-dev mailing list > >> >> mesa-dev@lists.freedesktop.org > >> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > >> > > >> > > >> > > > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
