On Tue, Jan 17, 2012 at 10:50 AM, Peter Maydell <peter.mayd...@linaro.org> wrote: > 2012/1/13 James Greensky <gsk...@gmail.com>: >> Sure, usually a tb chain is setup after a subsequent tb is >> found/constructed in the loop in cpu_exec when a tb returns. >> Taken/non-taken branch chaining is implemented by indicating the >> branch direction by the two least significant digits of the the >> previously returned tb. This is usually 0/1. When running icount, you >> can also get a 2 value in these least significant digits, indicating >> that the translation block was restarted due to the >> icount_decr.u16.low field being exhausted but having instructions left >> to execute in icount_extra. This 2 value falls through to tb_add_jump, >> which then updates the tb's jmp_first field, as both tb and next_tb >> refer to the same translation block. My question is why is this >> necessary, why not do nothing, and leave the previous chaining intact? > > This looks suspiciously like a bug to me, although the code > is a bit opaque to me. Calling tb_add_jump() with n==2 seems like > it's not going to work since tb_set_jmp_target() is going to fall > off the end of either tb_jmp_offset[] or tb_next[], so even if we're > playing clever games with jmp_first being treatable as jmp_next[2] for > some purposes there's going to be a problem. > > I thought the 2 return from a TB execution was only used as a flag > value, which would suggest that we should clear next_tb in the > 'refill decrementer' code path too. I'm not sure I'm not missing > something subtle, though. > > -- PMM
I agree, seems like a bug, but wanted to verify with the author or somebody that understands that section of the code whether this is intentional or not. -Jim
