On Thu, Jan 12, 2017 at 12:38 PM, Matt Turner <matts...@gmail.com> wrote:
> On Thu, Dec 1, 2016 at 1:51 PM, Jason Ekstrand <ja...@jlekstrand.net>
> wrote:
> > This fixes a bug in code motion that occurred when the best block is the
> > same as the schedule early block. In this case, because we're checking
> > (lca != def->parent_instr->block) at the top of the loop, we never get to
> > the check for loop depth so we wouldn't move it out of the loop. This
> > commit reworks the loop to be a simple for loop up the dominator chain
> and
> > we place the (lca != def->parent_instr->block) check at the end of the
> > loop.
> > ---
> > src/compiler/nir/nir_opt_gcm.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/compiler/nir/nir_opt_gcm.c b/src/compiler/nir/nir_opt_
> gcm.c
> > index 77eb8e6..9d7f59c 100644
> > --- a/src/compiler/nir/nir_opt_gcm.c
> > +++ b/src/compiler/nir/nir_opt_gcm.c
> > @@ -326,12 +326,13 @@ gcm_schedule_late_def(nir_ssa_def *def, void
> *void_state)
> > * as far outside loops as we can get.
>
> In this comment, the first line says "know" instead of "now". Please
> fix that while we're here.
>
Thanks. I'll fix that up.
> > */
> > nir_block *best = lca;
> > - while (lca != def->parent_instr->block) {
> > - assert(lca);
> > - if (state->blocks[lca->index].loop_depth <
> > + for (nir_block *block = lca; block != NULL; block = block->imm_dom) {
> > + if (state->blocks[block->index].loop_depth <
> > state->blocks[best->index].loop_depth)
> > - best = lca;
> > - lca = lca->imm_dom;
> > + best = block;
> > +
> > + if (block == def->parent_instr->block)
> > + break;
> > }
>
> I had to come up with an example and walk through it to understand the
> issue, but I now understand and agree that this is the correct fix.
>
> This patch is
>
> Reviewed-by: Matt Turner <matts...@gmail.com>
>
Thanks! I'd almost forgotten about this patch.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
