From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36900) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buPbX-0004sd-DV for qemu-devel@nongnu.org; Wed, 12 Oct 2016 15:54:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buPbR-00073y-A8 for qemu-devel@nongnu.org; Wed, 12 Oct 2016 15:54:18 -0400 Received: from mail-pa0-x22f.google.com ([2607:f8b0:400e:c03::22f]:33649) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buPbQ-00073O-Qa for qemu-devel@nongnu.org; Wed, 12 Oct 2016 15:54:13 -0400 Received: by mail-pa0-x22f.google.com with SMTP id vu5so31105857pab.0 for ; Wed, 12 Oct 2016 12:54:12 -0700 (PDT) References: <1474047287-145701-1-git-send-email-thomas.hanson@linaro.org> <1474047287-145701-3-git-send-email-thomas.hanson@linaro.org> <57F57641.3090104@linaro.org> From: Tom Hanson Message-ID: <57FE9497.2000907@linaro.org> Date: Wed, 12 Oct 2016 13:52:55 -0600 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] target-arm: Code changes to implement overwrite of tag field on PC load List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers , Grant Likely , Richard Henderson On 10/11/2016 10:12 AM, Peter Maydell wrote: > On 11 October 2016 at 16:51, Thomas Hanson wrote: >> On 5 October 2016 at 16:01, Peter Maydell wrote: >>> It matches the style of the rest of the code which generally >>> prefers to convert register numbers into TCGv earlier rather >>> than later (at the level which is doing decode of instruction >>> bits, rather than inside utility functions), and gives you a >>> more flexible utility function, which can do a "write value to PC" >>> for any value, not just something that happens to be in a CPU >>> register. And as you say it avoids calling cpu_reg() multiple times >>> as a side benefit. > >> This approach seems counter to both structured and OO design principles >> which would push common code (like type conversion) down into the lower >> level function in order to increase re-use and minimize code duplication. >> Those principles suggest that if we need a gen_a64_set_pc_value() function >> that can load the PC from something other than a register or an immediate, >> then it should be a lower level function than, and be called by, >> gen_a64_set_pc_reg(). This also has the benefit of reducing clutter in the >> caller, making it more readable and more maintainable. > > The 'lower level' stuff here has a general pattern of taking either > (1) a TCGv or (2) an integer immediate. We should follow that pattern. > >> As a separate issue, we now have functions to load the PC from an immediate >> value and from a register. Where else could we legitimately load the PC >> from? > > Anything where we found ourselves wanting to do some preliminary > manipulation of the value before writing it to the PC. > > thanks > -- PMM > I split gen_a64_set_pc_reg() into 2 funtions, upper that takes a register and lower that takes a variable. Patch v3 submitted.