sergey.fedo...@linaro.org writes: > From: Paolo Bonzini <pbonz...@redhat.com> > > Use a continue statement. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > [Sergey Fedorov: Fix moving to list head in case of no TB] > Signed-off-by: Sergey Fedorov <sergey.fedo...@linaro.org> > --- > cpu-exec.c | 50 +++++++++++++++++++++++++------------------------- > 1 file changed, 25 insertions(+), 25 deletions(-) > > diff --git a/cpu-exec.c b/cpu-exec.c > index fd92452f16f6..f90482eff778 100644 > --- a/cpu-exec.c > +++ b/cpu-exec.c > @@ -225,37 +225,37 @@ static TranslationBlock *tb_find_physical(CPUState *cpu, > phys_pc = get_page_addr_code(env, pc); > phys_page1 = phys_pc & TARGET_PAGE_MASK; > h = tb_phys_hash_func(phys_pc); > - ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h]; > - for(;;) { > - tb = *ptb1; > - if (!tb) { > - return NULL; > + for (ptb1 = &tcg_ctx.tb_ctx.tb_phys_hash[h]; > + (tb = *ptb1) != NULL; > + ptb1 = &tb->phys_hash_next) {
I'm not sure I'm keen on the assignment in the for condition clause. I appreciate the cleansing of the if !tb return exit though. Could we be cleaner maybe? Here is my attempt: static TranslationBlock *tb_find_physical(CPUState *cpu, target_ulong pc, target_ulong cs_base, uint64_t flags) { CPUArchState *env = (CPUArchState *)cpu->env_ptr; TranslationBlock *tb, **tb_hash_head, **ptb1; unsigned int h; tb_page_addr_t phys_pc, phys_page1; /* find translated block using physical mappings */ phys_pc = get_page_addr_code(env, pc); phys_page1 = phys_pc & TARGET_PAGE_MASK; h = tb_phys_hash_func(phys_pc); /* Start at head of the hash entry */ ptb1 = tb_hash_head = &tcg_ctx.tb_ctx.tb_phys_hash[h]; tb = *ptb1; while (tb) { if (tb->pc == pc && tb->page_addr[0] == phys_page1 && tb->cs_base == cs_base && tb->flags == flags) { if (tb->page_addr[1] == -1) { /* done, we have a match */ break; } else { /* check next page if needed */ target_ulong virt_page2 = (pc & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; tb_page_addr_t phys_page2 = get_page_addr_code(env, virt_page2); if (tb->page_addr[1] == phys_page2) { break; } } } ptb1 = &tb->phys_hash_next; tb = *ptb1; } if (tb) { /* Move the TB to the head of the list */ *ptb1 = tb->phys_hash_next; tb->phys_hash_next = *tb_hash_head; *tb_hash_head = tb; } return tb; } FWIW the compiled code is 9 bytes shorter on my machine. > + if (tb->pc != pc || > + tb->page_addr[0] != phys_page1 || > + tb->cs_base != cs_base || > + tb->flags != flags) { > + continue; > } > - if (tb->pc == pc && > - tb->page_addr[0] == phys_page1 && > - tb->cs_base == cs_base && > - tb->flags == flags) { > - /* check next page if needed */ > - if (tb->page_addr[1] != -1) { > - tb_page_addr_t phys_page2; > - > - virt_page2 = (pc & TARGET_PAGE_MASK) + > - TARGET_PAGE_SIZE; > - phys_page2 = get_page_addr_code(env, virt_page2); > - if (tb->page_addr[1] == phys_page2) { > - break; > - } > - } else { > + > + /* check next page if needed */ > + if (tb->page_addr[1] != -1) { > + tb_page_addr_t phys_page2; > + > + virt_page2 = (pc & TARGET_PAGE_MASK) + > + TARGET_PAGE_SIZE; > + phys_page2 = get_page_addr_code(env, virt_page2); > + if (tb->page_addr[1] == phys_page2) { > break; > } > + } else { > + break; > } > - ptb1 = &tb->phys_hash_next; > } > > - /* Move the TB to the head of the list */ > - *ptb1 = tb->phys_hash_next; > - tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h]; > - tcg_ctx.tb_ctx.tb_phys_hash[h] = tb; > + if (tb) { > + /* Move the TB to the head of the list */ > + *ptb1 = tb->phys_hash_next; > + tb->phys_hash_next = tcg_ctx.tb_ctx.tb_phys_hash[h]; > + tcg_ctx.tb_ctx.tb_phys_hash[h] = tb; > + } > return tb; > } -- Alex Bennée