Re: [Mesa-dev] [PATCH 1/5] i965/vec4: nir_emit_if doesn't need to predicate based on all the channels
On Sat, Oct 10, 2015 at 4:24 AM, Alejandro Piñeiro wrote: > --- > > I already talked about this with Jason Ekstrand and Matt Turner > privately, but just in case somebody else jump to the review: > > When using BRW_PREDICATE_NORMAL, the if will use all the channels of > the register flag. But nir_if only reads from one channel, so that > is not needed. Another hint showing that this is safe: the MOV that > put the condition on f0 is calling get_nir_src with just one > component. That will return always a source with swizzle > BRW_SWIZZLE_, so that component is the only to be used. > > This commit is not needed/solving anything per-se, but it is needed in > order to be able to implement vec4_cmod_propagation with a good > overall outcome. > > src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 4 +++- > 1 file changed, 3 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 41bd80d..e05745f 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > @@ -193,7 +193,9 @@ vec4_visitor::nir_emit_if(nir_if *if_stmt) > vec4_instruction *inst = emit(MOV(dst_null_d(), condition)); > inst->conditional_mod = BRW_CONDITIONAL_NZ; > > - emit(IF(BRW_PREDICATE_NORMAL)); > + /* We can just predicate based on the X channel, as the condition only > +* reads from one channel */ */ goes on its own line. > + emit(IF(BRW_PREDICATE_ALIGN16_REPLICATE_X)); I agree with what Jason says -- seems like we should have been doing this all along. Reviewed-by: Matt Turner ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/5] i965/vec4: nir_emit_if doesn't need to predicate based on all the channels
Looking at the docs a bit, it looks like we should never have been using predicate_normal for if's in the first place Reviewed-by: Jason Ekstrand On Sat, Oct 10, 2015 at 4:24 AM, Alejandro Piñeiro wrote: > --- > > I already talked about this with Jason Ekstrand and Matt Turner > privately, but just in case somebody else jump to the review: > > When using BRW_PREDICATE_NORMAL, the if will use all the channels of > the register flag. But nir_if only reads from one channel, so that > is not needed. Another hint showing that this is safe: the MOV that > put the condition on f0 is calling get_nir_src with just one > component. That will return always a source with swizzle > BRW_SWIZZLE_, so that component is the only to be used. > > This commit is not needed/solving anything per-se, but it is needed in > order to be able to implement vec4_cmod_propagation with a good > overall outcome. > > src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 4 +++- > 1 file changed, 3 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 41bd80d..e05745f 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > @@ -193,7 +193,9 @@ vec4_visitor::nir_emit_if(nir_if *if_stmt) > vec4_instruction *inst = emit(MOV(dst_null_d(), condition)); > inst->conditional_mod = BRW_CONDITIONAL_NZ; > > - emit(IF(BRW_PREDICATE_NORMAL)); > + /* We can just predicate based on the X channel, as the condition only > +* reads from one channel */ > + emit(IF(BRW_PREDICATE_ALIGN16_REPLICATE_X)); > > nir_emit_cf_list(&if_stmt->then_list); > > -- > 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] [PATCH 1/5] i965/vec4: nir_emit_if doesn't need to predicate based on all the channels
--- I already talked about this with Jason Ekstrand and Matt Turner privately, but just in case somebody else jump to the review: When using BRW_PREDICATE_NORMAL, the if will use all the channels of the register flag. But nir_if only reads from one channel, so that is not needed. Another hint showing that this is safe: the MOV that put the condition on f0 is calling get_nir_src with just one component. That will return always a source with swizzle BRW_SWIZZLE_, so that component is the only to be used. This commit is not needed/solving anything per-se, but it is needed in order to be able to implement vec4_cmod_propagation with a good overall outcome. src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 4 +++- 1 file changed, 3 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 41bd80d..e05745f 100644 --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp @@ -193,7 +193,9 @@ vec4_visitor::nir_emit_if(nir_if *if_stmt) vec4_instruction *inst = emit(MOV(dst_null_d(), condition)); inst->conditional_mod = BRW_CONDITIONAL_NZ; - emit(IF(BRW_PREDICATE_NORMAL)); + /* We can just predicate based on the X channel, as the condition only +* reads from one channel */ + emit(IF(BRW_PREDICATE_ALIGN16_REPLICATE_X)); nir_emit_cf_list(&if_stmt->then_list); -- 2.1.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
