From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49297) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btzg8-0000Xn-K0 for qemu-devel@nongnu.org; Tue, 11 Oct 2016 12:13:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1btzg6-0000xU-H0 for qemu-devel@nongnu.org; Tue, 11 Oct 2016 12:13:19 -0400 Received: from mail-vk0-x232.google.com ([2607:f8b0:400c:c05::232]:36405) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1btzg6-0000xL-2A for qemu-devel@nongnu.org; Tue, 11 Oct 2016 12:13:18 -0400 Received: by mail-vk0-x232.google.com with SMTP id 2so24353055vkb.3 for ; Tue, 11 Oct 2016 09:13:18 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: 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: Peter Maydell Date: Tue, 11 Oct 2016 17:12:57 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 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: Thomas Hanson Cc: QEMU Developers , Grant Likely , Richard Henderson 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