On Thu, Nov 19, 2015 at 2:05 PM, Kenneth Graunke <kenn...@whitecape.org> wrote: > We've apparently always been botching JIP for sequences such as:
Wrong since Dec 1 2015! Nice find. > > do > cmp.f0.0 ... > (+f0.0) break > ... > if > ... > else > ... > endif > ... > while > > Normally, UIP is supposed to point to the final destination of the jump, > while in nested control flow, JIP is supposed to point to the end of the > current nesting level. It essentially bounces out of the current nested > control flow, to an instruction that has a JIP which bounces out another > level, and so on. > > In the above example, when setting JIP for the BREAK, we call > brw_find_next_block_end(), which begins a search after the BREAK for the > next ENDIF, ELSE, WHILE, or HALT. It ignores the IF and finds the ELSE, > setting JIP there. > > This makes no sense at all. The break is supposed to skip over the > whole if/else/endif block entirely. They have a sibling relationship, > not a nesting relationship. > > This patch fixes brw_find_next_block_end() to track depth as it does > its search, and ignore anything not at depth 0. So when it sees the > IF, it ignores everything until after the ENDIF. That way, it finds > the end of the right block. > > Caught while debugging a tessellation shader - no apparent effect on > Piglit. I did look for actual applications that were affected, and > found that GLBenchmark Manhattan had a BREAK with a bogus JIP. > > Cc: mesa-sta...@lists.freedesktop.org > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/mesa/drivers/dri/i965/brw_eu_emit.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c > b/src/mesa/drivers/dri/i965/brw_eu_emit.c > index da1ddfd..e457fd2 100644 > --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c > +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c > @@ -2604,17 +2604,27 @@ brw_find_next_block_end(struct brw_codegen *p, int > start_offset) > void *store = p->store; > const struct brw_device_info *devinfo = p->devinfo; > > + int depth = 0; > + > for (offset = next_offset(devinfo, store, start_offset); > offset < p->next_insn_offset; > offset = next_offset(devinfo, store, offset)) { > brw_inst *insn = store + offset; > > switch (brw_inst_opcode(devinfo, insn)) { > + case BRW_OPCODE_IF: > + depth++; > + break; > case BRW_OPCODE_ENDIF: > + if (depth == 0) > + return offset; > + depth--; > + break; > case BRW_OPCODE_ELSE: > case BRW_OPCODE_WHILE: > case BRW_OPCODE_HALT: > - return offset; > + if (depth == 0) > + return offset; Reviewed-by: Matt Turner <matts...@gmail.com> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev