From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40397) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZfDN9-0000fw-AE for qemu-devel@nongnu.org; Thu, 24 Sep 2015 16:44:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZfDN5-0008DT-9F for qemu-devel@nongnu.org; Thu, 24 Sep 2015 16:44:07 -0400 Received: from mail-qg0-x231.google.com ([2607:f8b0:400d:c04::231]:33881) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZfDN5-0008Cb-65 for qemu-devel@nongnu.org; Thu, 24 Sep 2015 16:44:03 -0400 Received: by qgez77 with SMTP id z77so54985996qge.1 for ; Thu, 24 Sep 2015 13:44:02 -0700 (PDT) Sender: Richard Henderson References: <1442953507-4074-1-git-send-email-rth@twiddle.net> <1442953507-4074-19-git-send-email-rth@twiddle.net> <20150924200252.GG12978@aurel32.net> From: Richard Henderson Message-ID: <5604608E.2070406@twiddle.net> Date: Thu, 24 Sep 2015 13:43:58 -0700 MIME-Version: 1.0 In-Reply-To: <20150924200252.GG12978@aurel32.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 18/25] tcg: Add TCG_MAX_INSNS List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: peter.maydell@linaro.org, alex.bennee@linaro.org, qemu-devel@nongnu.org On 09/24/2015 01:02 PM, Aurelien Jarno wrote: >> @@ -2903,6 +2903,9 @@ static inline void gen_intermediate_code_internal(AlphaCPU *cpu, >> if (max_insns == 0) { >> max_insns = CF_COUNT_MASK; >> } > > I guess you can change also change the value to TCG_MAX_INSNS, though I > guess the compiler will realize about that. I did wonder about the best thing to do re CF_COUNT_MASK. Especially as it's currently set to 0x7fff. FWIW, the largest TB I've seen so far while collecting statistics is 157 insns. So the current setting of TCG_MAX_INSNS at 512 is more than enough. > >> + if (max_insns > TCG_MAX_INSNS) { >> + max_insns = TCG_MAX_INSNS; >> + } >> >> if (in_superpage(&ctx, pc_start)) { >> pc_mask = (1ULL << 41) - 1; > > Given we have the same pattern in all targets, I do wonder if it > wouldn't be better to just setup (cflags & CF_COUNT_MASK) to > TCG_MAX_INSNS instead of 0 in translate-all.c when not using icount. Yes, that would probably be best. There should probably be some helper function that handles all these as well as noticing single-stepping. Too many targets test (num_insns >= max_insns || singlestep || ...) when we could just as well set max_insns to 1 and have just the one runtime test. Then there's all the targets which have a fixed insn size, where we can pre-compute the number of insns left on the page, and fold in the end-of-page test as well. I'll put cleaning this up on the to-do list. r~ > > That said your code is correct, so: > > Reviewed-by: Aurelien Jarno >