From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53493) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aKCu3-00064n-5V for qemu-devel@nongnu.org; Fri, 15 Jan 2016 17:31:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aKCtz-0005Ij-Rs for qemu-devel@nongnu.org; Fri, 15 Jan 2016 17:31:31 -0500 Received: from mail-qk0-x244.google.com ([2607:f8b0:400d:c09::244]:35180) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aKCtz-0005II-ND for qemu-devel@nongnu.org; Fri, 15 Jan 2016 17:31:27 -0500 Received: by mail-qk0-x244.google.com with SMTP id s5so1015033qkd.2 for ; Fri, 15 Jan 2016 14:31:27 -0800 (PST) Sender: Richard Henderson References: <1450382320-5383-1-git-send-email-rth@twiddle.net> <1450382429-6019-1-git-send-email-rth@twiddle.net> <20151231115430.GF360@aurel32.net> From: Richard Henderson Message-ID: <5699733B.30404@twiddle.net> Date: Fri, 15 Jan 2016 14:31:23 -0800 MIME-Version: 1.0 In-Reply-To: <20151231115430.GF360@aurel32.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 11/14] tcg: Implement indirect memory registers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: mark.cave-ayland@ilande.co.uk, qemu-devel@nongnu.org On 12/31/2015 03:54 AM, Aurelien Jarno wrote: > On 2015-12-17 12:00, Richard Henderson wrote: >> That is, global_mem registers whose base is another global_mem >> register, rather than a fixed register. >> >> Signed-off-by: Richard Henderson >> --- >> tcg/tcg.c | 95 ++++++++++++++++++++++++++++++++++++++++++++------------------- >> tcg/tcg.h | 2 ++ >> 2 files changed, 68 insertions(+), 29 deletions(-) >> >> diff --git a/tcg/tcg.c b/tcg/tcg.c >> index c51e0ec..7150a3f 100644 >> --- a/tcg/tcg.c >> +++ b/tcg/tcg.c >> @@ -509,17 +509,23 @@ int tcg_global_mem_new_internal(TCGType type, TCGv_ptr base, >> TCGContext *s = &tcg_ctx; >> TCGTemp *base_ts = &s->temps[GET_TCGV_PTR(base)]; >> TCGTemp *ts = tcg_global_alloc(s); >> - int bigendian = 0; >> + int indirect_reg = 0, bigendian = 0; >> #ifdef HOST_WORDS_BIGENDIAN >> bigendian = 1; >> #endif >> >> + if (!base_ts->fixed_reg) { >> + indirect_reg = 1; >> + base_ts->indirect_base = 1; >> + } >> + >> if (TCG_TARGET_REG_BITS == 32 && type == TCG_TYPE_I64) { >> TCGTemp *ts2 = tcg_global_alloc(s); >> char buf[64]; >> >> ts->base_type = TCG_TYPE_I64; >> ts->type = TCG_TYPE_I32; >> + ts->indirect_reg = indirect_reg; > > Do we really need to add this new bit, while we can simply test > ts->mem_base->fixed_reg? This means one more derefence, but anyway it > has to be done later when calling temp_load? I thought it cleaner to have the bit. We're using 7 of 32 (or 39 of 64) from the word, so we're not really short of bit space. >> + /* ??? Liveness does not yet incorporate indirect bases. */ >> + if (!ts->indirect_base) { >> + /* The liveness analysis already ensures that globals are back >> + in memory. Keep an assert for safety. */ >> + tcg_debug_assert(ts->val_type == TEMP_VAL_MEM || ts->fixed_reg); >> + return; >> + } > > This basically disables the assert. What does it mean in practice? Can > it generates bad code? Since we have no liveness info for the indirect base, we don't know exactly where it begins and ends life. That could be fixed with additional effort, but it didn't seem to be required during this initial implementation. Since we don't have liveness info, I simply want to follow the path we would take when liveness info isn't computed. We do not generate bad code. r~