From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58577) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj6mF-0001y3-Fc for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:02:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aj6m9-0007X1-KT for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:02:23 -0400 Received: from mail-wm0-x232.google.com ([2a00:1450:400c:c09::232]:37839) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj6m9-0007Ww-9T for qemu-devel@nongnu.org; Thu, 24 Mar 2016 11:02:17 -0400 Received: by mail-wm0-x232.google.com with SMTP id p65so69890842wmp.0 for ; Thu, 24 Mar 2016 08:02:17 -0700 (PDT) References: <1458815961-31979-1-git-send-email-sergey.fedorov@linaro.org> <1458815961-31979-2-git-send-email-sergey.fedorov@linaro.org> <87poukq9fk.fsf@linaro.org> <56F3F377.4070809@gmail.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <56F3F377.4070809@gmail.com> Date: Thu, 24 Mar 2016 15:01:37 +0000 Message-ID: <87mvpnrkby.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1/8] tcg: Clean up direct block chaining data fields List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Fedorov Cc: sergey.fedorov@linaro.org, Peter Crosthwaite , Stefan Weil , Claudio Fontana , qemu-devel@nongnu.org, Alexander Graf , Blue Swirl , qemu-arm@nongnu.org, "Vassili Karpov (malc)" , Paolo Bonzini , Aurelien Jarno , Richard Henderson Sergey Fedorov writes: > On 24/03/16 16:42, Alex Bennée wrote: >>> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h >>> > index 05a151da4a54..cc3d2ca25917 100644 >>> > --- a/include/exec/exec-all.h >>> > +++ b/include/exec/exec-all.h >>> > @@ -257,20 +257,32 @@ struct TranslationBlock { >>> > struct TranslationBlock *page_next[2]; >>> > tb_page_addr_t page_addr[2]; >>> > >>> > - /* the following data are used to directly call another TB from >>> > - the code of this one. */ >>> > - uint16_t tb_next_offset[2]; /* offset of original jump target */ >>> > + /* The following data are used to directly call another TB from >>> > + * the code of this one. This can be done either by emitting direct or >>> > + * indirect native jump instructions. These jumps are reset so that the TB >>> > + * just continue its execution. The TB can be linked to another one by >>> > + * setting one of the jump targets (or patching the jump instruction). Only >>> > + * two of such jumps are supported. >>> > + */ >>> > + uint16_t jmp_reset_offset[2]; /* offset of original jump target */ >>> > +#define TB_JMP_RESET_OFFSET_INVALID 0xffff /* indicates no jump generated */ >>> > #ifdef USE_DIRECT_JUMP >>> > - uint16_t tb_jmp_offset[2]; /* offset of jump instruction */ >>> > + uint16_t jmp_insn_offset[2]; /* offset of native jump instruction */ >>> > #else >>> > - uintptr_t tb_next[2]; /* address of jump generated code */ >>> > + uintptr_t jmp_target_addr[2]; /* target address for indirect jump */ >>> > #endif >>> > - /* list of TBs jumping to this one. This is a circular list using >>> > - the two least significant bits of the pointers to tell what is >>> > - the next pointer: 0 = jmp_next[0], 1 = jmp_next[1], 2 = >>> > - jmp_first */ >>> > - struct TranslationBlock *jmp_next[2]; >>> > - struct TranslationBlock *jmp_first; >>> > + /* Each TB has an assosiated circular list of TBs jumping to this one. >>> > + * jmp_list_first points to the first TB jumping to this one. >>> > + * jmp_list_next is used to point to the next TB in a list. >>> > + * Since each TB can have two jumps, it can participate in two lists. >>> > + * The two least significant bits of a pointer are used to choose which >>> > + * data field holds a pointer to the next TB: >>> > + * 0 => jmp_list_next[0], 1 => jmp_list_next[1], 2 => jmp_list_first. >>> > + * In other words, 0/1 tells which jump is used in the pointed TB, >>> > + * and 2 means that this is a pointer back to the target TB of this list. >>> > + */ >>> > + struct TranslationBlock *jmp_list_next[2]; >>> > + struct TranslationBlock *jmp_list_first; >> OK I found that tricky to follow. Where does the value of the pointer >> come from that sets these bottom bits? The TB jumping to this TB sets it? > > Yeah, that's not easy to describe. Initially, we set: > > tb->jmp_list_first = tb | 2 > > That makes an empty list: jmp_list_first just points to the this TB and > the low bits are 2. > > After that we can add a TB to the list in tb_add_jump(): > > tb->jmp_list_next[n] = tb_next->jmp_list_first; > tb_next->jmp_list_first = tb | n; > > where 'tb' is going to jump to 'tb_next', 'n' (can be 0 or 1) is an > index of jump target of 'tb'. Where I get confused it what is the point of jmp_list_first? If these are two circular lists do we care which the first in the list is? The exit condition when coming out of searching seems when ntb with index = orig tb with index. > > (I simplified the code here) > > Any ideas how to make it more clear in the comment? > > Kind regards, > Sergey -- Alex Bennée