From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=46009 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q6Ku1-0005in-S0 for qemu-devel@nongnu.org; Sun, 03 Apr 2011 06:51:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q6Ktw-0007KD-SA for qemu-devel@nongnu.org; Sun, 03 Apr 2011 06:51:29 -0400 Received: from mail-vx0-f173.google.com ([209.85.220.173]:57043) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q6Ktw-0007Jq-PW for qemu-devel@nongnu.org; Sun, 03 Apr 2011 06:51:24 -0400 Received: by vxb41 with SMTP id 41so4130042vxb.4 for ; Sun, 03 Apr 2011 03:51:23 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1301668243-29886-1-git-send-email-peter.maydell@linaro.org> <1301668243-29886-2-git-send-email-peter.maydell@linaro.org> Date: Sun, 3 Apr 2011 11:51:23 +0100 Message-ID: Subject: Re: [Qemu-devel] [PATCH 01/10] target-arm: Make Neon helper routines use correct FP status From: Peter Maydell Content-Type: text/plain; charset=UTF-8 List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Aurelien Jarno , qemu-devel@nongnu.org On 3 April 2011 10:41, Blue Swirl wrote: > On Sat, Apr 2, 2011 at 1:33 AM, Peter Maydell wrote: >> Nobody seemed to have a coherent rule about when functions >> should be in op_helper.c and use the global 'env' variable >> and when they should be in some other file and have 'env' >> passed as a parameter > In general, helpers for the translated code belong to op_helper.c. > They can and should access global env directly for speed. If a helper > is used for both translated code and outside of it, a wrapper should > be added to do global env shuffling (or for example a copy without > shuffling added). OK, we can do that, but at the moment "helper function not in op_helper.c" is hugely in the majority so there's a lot of code we'd be moving around: $ grep -c HELPER target-arm/*.c target-arm/helper.c:68 target-arm/iwmmxt_helper.c:59 target-arm/machine.c:0 target-arm/neon_helper.c:103 target-arm/op_helper.c:28 target-arm/translate.c:2 (ie just 10% or so of ARM helper functions are in op_helper.c) ...and this cleanup would basically amount to folding neon_helper.c, iwmmxt_helper.c and bits of helper.c into op_helper.c (and then removing the 'env' parameters, so a big patch to translate.c as well, which I don't suppose anybody maintaining an out-of-tree target-arm patchset will thank us for :-)). But I can submit a patch to do that if it's the right thing. -- PMM