From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45714) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THJvZ-0006un-Ut for qemu-devel@nongnu.org; Thu, 27 Sep 2012 15:39:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1THJvV-0000If-JS for qemu-devel@nongnu.org; Thu, 27 Sep 2012 15:39:17 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:34392) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THJvV-0000IX-BV for qemu-devel@nongnu.org; Thu, 27 Sep 2012 15:39:13 -0400 Received: by pbbrp2 with SMTP id rp2so4071229pbb.4 for ; Thu, 27 Sep 2012 12:39:12 -0700 (PDT) Sender: Richard Henderson Message-ID: <5064AB5E.2010006@twiddle.net> Date: Thu, 27 Sep 2012 12:39:10 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1348766113-18373-1-git-send-email-aurelien@aurel32.net> <1348766113-18373-12-git-send-email-aurelien@aurel32.net> In-Reply-To: <1348766113-18373-12-git-send-email-aurelien@aurel32.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 11/13] tcg: sync globals for pure helpers instead of saving them List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: qemu-devel@nongnu.org On 09/27/2012 10:15 AM, Aurelien Jarno wrote: > Note: this is a change of behavior, but the new one is still compliant > with the documentation. Currently only cris, i386 and s390x have PURE > only helpers. I have checked they are really PURE, and also did some > tests on cris and i386. > > diff --git a/tcg/README b/tcg/README > index 27846f1..d61daa1 100644 > --- a/tcg/README > +++ b/tcg/README > @@ -77,11 +77,16 @@ destroyed, but local temporaries and globals are preserved. > Using the tcg_gen_helper_x_y it is possible to call any function > taking i32, i64 or pointer types. By default, before calling a helper, > all globals are stored at their canonical location and it is assumed > -that the function can modify them. This can be overridden by the > -TCG_CALL_CONST function modifier. By default, the helper is allowed to > -modify the CPU state or raise an exception. This can be overridden by > -the TCG_CALL_PURE function modifier, in which case the call to the > -function is removed if the return value is not used. > +that the function can modify them. The helper is allowed to modify > +the CPU state or raise an exception. > + > +This can be overridden by the TCG_CALL_CONST and TCG_CALL_PURE function > +modifiers. A PURE helper can read globals but cannot modify them nor the > +CPU state and cannot raise an exception. It can be removed if the return > +value is not used. For that the globals are synchronized with their canonical > +location, but the associated registers are not freed nor reloaded afterwise. > +A CONST helper is a PURE helper which in addition cannot read globals, they > +are not synchronized nor stored. Note that CONST implies PURE. If we're going to re-org these flags, lets please do it right. In particular the terms "const" and "pure" were clearly stolen from gcc, but then given different meanings (!). This, to me at least, is incredibly confusing. I have to go back and re-read the docs every time I touch one of the helper declarations. There are really three flags we care about: (1) Helper may read globals. Sometimes indirectly via exception. Implies that globals must be synced, but may remain in REG/CONST state. (2) Helper may write globals. Implies globals must be synced, and returned to MEM state. (3) Helper has no side effects at all. Implies that it can be deleted if its outputs are dead. For the sake of discussion, lets call these READG, WRITEG, NOSIDE. Our current default is READG | WRITEG. Our current definition of PURE is READG | NOSIDE. Note that the gcc definition of pure actually talks about main memory. Our current definition of CONST is none of these bits. Note that the gcc definition of const is a superset of pure. What we'd like for all fp helpers is READG only. Something that we cannot get at the moment. There are several cases in which the helper needs to return more than 64 bits in which we use a "temp" allocated in the env struct. Sparc does this for some of its 128-bit arithmetic; I'm planning to do just the same in my s390x rewrite. For this, the helper neither reads nor writes globals, but it has non-global-register side effects. There will generally be a post-helper load from env to get the "extra" bits. While the "true" output of the helper may be dead, the side load from env may not be. So we want none of READG | WRITEG | NOSIDE. If we do reorg, we can certainly add compatibility defines so that we need not update all translators at once. r~