Sergey Fedorov <serge.f...@gmail.com> writes: > On 24/03/16 18:11, Alex Bennée wrote: >> sergey.fedo...@linaro.org writes: >>> From: Sergey Fedorov <serge.f...@gmail.com> >>> >>> diff --git a/translate-all.c b/translate-all.c >>> index ca01dd325b8d..f68716e1819f 100644 >>> --- a/translate-all.c >>> +++ b/translate-all.c >>> @@ -1131,19 +1131,6 @@ static void tb_link_page(TranslationBlock *tb, >>> tb_page_addr_t phys_pc, >>> tb->page_addr[1] = -1; >>> } >>> >>> - assert(((uintptr_t)tb & 3) == 0); >>> - tb->jmp_list_first = (uintptr_t)tb | 2; >>> - tb->jmp_list_next[0] = (uintptr_t)NULL; >>> - tb->jmp_list_next[1] = (uintptr_t)NULL; >>> - >>> - /* init original jump addresses */ >>> - if (tb->jmp_reset_offset[0] != TB_JMP_RESET_OFFSET_INVALID) { >>> - tb_reset_jump(tb, 0); >>> - } >>> - if (tb->jmp_reset_offset[1] != TB_JMP_RESET_OFFSET_INVALID) { >>> - tb_reset_jump(tb, 1); >>> - } >>> - >>> #ifdef DEBUG_TB_CHECK >>> tb_page_check(); >>> #endif >>> @@ -1251,6 +1238,20 @@ TranslationBlock *tb_gen_code(CPUState *cpu, >>> ROUND_UP((uintptr_t)gen_code_buf + gen_code_size + search_size, >>> CODE_GEN_ALIGN); >>> >>> + /* init jump list */ >>> + assert(((uintptr_t)tb & 3) == 0); >>> + tb->jmp_list_first = (uintptr_t)tb | 2; >>> + tb->jmp_list_next[0] = (uintptr_t)NULL; >>> + tb->jmp_list_next[1] = (uintptr_t)NULL; >> maybe these should be further up the function with the other jmp setting >> code? > > I meant to keep them together with the following lines. > >> >>> + >>> + /* init original jump addresses */ >>> + if (tb->jmp_reset_offset[0] != TB_JMP_RESET_OFFSET_INVALID) { >>> + tb_reset_jump(tb, 0); >>> + } >>> + if (tb->jmp_reset_offset[1] != TB_JMP_RESET_OFFSET_INVALID) { >>> + tb_reset_jump(tb, 1); >>> + } >> Why would tb->jmp_reset_offset[0] == TB_JMP_RESET_OFFSET_INVALID not be >> the case as it is set a few lines further up. > > Because tcg_gen_code() gets called in between and it is passed > '&tcg_ctx' which holds a pointer to 'tb->jmp_reset_offset'. tcg_out_op() > is called from tcg_gen_code() and sets 'tb->jmp_reset_offset[n]' > indirectly, as well as 'tb->jmp_insn_offset[n]'.
OK a quick addition to the comment: "these may have been reset in tcg_gen_code" would be helpful here. > > Kind regards, > Sergey -- Alex Bennée