From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54974) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aktfe-0005jz-Iz for qemu-devel@nongnu.org; Tue, 29 Mar 2016 09:26:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aktfb-0005dR-C8 for qemu-devel@nongnu.org; Tue, 29 Mar 2016 09:26:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44289) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aktfb-0005cq-4b for qemu-devel@nongnu.org; Tue, 29 Mar 2016 09:26:55 -0400 References: <1458222382-6498-1-git-send-email-sergey.fedorov@linaro.org> <1458222382-6498-3-git-send-email-sergey.fedorov@linaro.org> <87d1qmsgmv.fsf@linaro.org> <56FA80D0.5020606@gmail.com> From: Paolo Bonzini Message-ID: <56FA829A.3040903@redhat.com> Date: Tue, 29 Mar 2016 15:26:50 +0200 MIME-Version: 1.0 In-Reply-To: <56FA80D0.5020606@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/5] tcg: reorganize tb_find_physical loop List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Fedorov , =?UTF-8?Q?Alex_Benn=c3=a9e?= , sergey.fedorov@linaro.org Cc: Peter Crosthwaite , qemu-devel@nongnu.org, Richard Henderson On 29/03/2016 15:19, Sergey Fedorov wrote: > On 22/03/16 17:59, Alex Benn=C3=A9e wrote: >> sergey.fedorov@linaro.org writes: >> >>> From: Paolo Bonzini >>> >>> Use a continue statement. >>> >>> Signed-off-by: Paolo Bonzini >>> [Sergey Fedorov: Fix moving to list head in case of no TB] >>> Signed-off-by: Sergey Fedorov >>> --- >>> 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(CPUSt= ate *cpu, >>> phys_pc =3D get_page_addr_code(env, pc); >>> phys_page1 =3D phys_pc & TARGET_PAGE_MASK; >>> h =3D tb_phys_hash_func(phys_pc); >>> - ptb1 =3D &tcg_ctx.tb_ctx.tb_phys_hash[h]; >>> - for(;;) { >>> - tb =3D *ptb1; >>> - if (!tb) { >>> - return NULL; >>> + for (ptb1 =3D &tcg_ctx.tb_ctx.tb_phys_hash[h]; >>> + (tb =3D *ptb1) !=3D NULL; >>> + ptb1 =3D &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 =3D (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 =3D get_page_addr_code(env, pc); >> phys_page1 =3D phys_pc & TARGET_PAGE_MASK; >> h =3D tb_phys_hash_func(phys_pc); >> >> /* Start at head of the hash entry */ >> ptb1 =3D tb_hash_head =3D &tcg_ctx.tb_ctx.tb_phys_hash[h]; >> tb =3D *ptb1; >> >> while (tb) { >> >> if (tb->pc =3D=3D pc && >> tb->page_addr[0] =3D=3D phys_page1 && >> tb->cs_base =3D=3D cs_base && >> tb->flags =3D=3D flags) { >> >> if (tb->page_addr[1] =3D=3D -1) { >> /* done, we have a match */ >> break; >> } else { >> /* check next page if needed */ >> target_ulong virt_page2 =3D (pc & TARGET_PAGE_MASK= ) >> + TARGET_PAGE_SIZE; >> tb_page_addr_t phys_page2 =3D get_page_addr_code(e= nv, virt_page2); >> >> if (tb->page_addr[1] =3D=3D phys_page2) { >> break; >> } >> } >> } >> >> ptb1 =3D &tb->phys_hash_next; >> tb =3D *ptb1; >> } >> >> if (tb) { >> /* Move the TB to the head of the list */ >> *ptb1 =3D tb->phys_hash_next; >> tb->phys_hash_next =3D *tb_hash_head; >> *tb_hash_head =3D tb; >> } >> return tb; >> } >> >> FWIW the compiled code is 9 bytes shorter on my machine. >=20 > Looks like another attempt to rewrite it. I am wondering whom to > attribute as an author, then? :) I don't really care. :) Paolo