From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44436) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W7sFu-00080Y-P6 for qemu-devel@nongnu.org; Mon, 27 Jan 2014 14:54:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W7sFm-0007Jt-Ba for qemu-devel@nongnu.org; Mon, 27 Jan 2014 14:54:02 -0500 Message-ID: <52E6B946.1080006@gmail.com> Date: Mon, 27 Jan 2014 13:53:42 -0600 From: Tom Musta MIME-Version: 1.0 References: <1390845264-2532-1-git-send-email-tommusta@gmail.com> <1390845264-2532-6-git-send-email-tommusta@gmail.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 5/8] target-ppc: Load Quadword List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: PowerPC , QEMU Developers On 1/27/2014 12:55 PM, Alexander Graf wrote: > You're mixing two semantically separate things here. legal_in_user_mode doesn't really indicate that le_mode isn't usable. I'm sure if you just make this two if()'s with two separate bools that get assigned the same value gcc will be smart enough to optimize it just as well as this combined branch. > Hmmm ... I'm not sure that I see the problem. Perhaps the comment should be clearer. And I guess there is really no need to compute the legal_in_user_mode flag since it is only used once. Prior to ISA 2.07, lq was not legal in user mode; attempting to execute lq when MSR[PR]=1 resulted in a privileged instruction exception. Also, when MSR[PR]=0 and MSR[LE]=1, an alignment exception was generated irrespective of the computed address. Starting with ISA 2.07, both of these restrictions are lifted. So the proposed code is as follows: static void gen_lq(DisasContext *ctx) { /* lq is a legal user mode instruction starting in ISA 2.07 */ int legal_in_user_mode = (ctx->insns_flags2 & PPC2_LSQ_ISA207) != 0; if (!legal_in_user_mode) { #if defined(CONFIG_USER_ONLY) gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); return; #else if (unlikely(ctx->mem_idx == 0)) { gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC); return; } if (unlikely(ctx->le_mode)) { /* Little-endian mode is not handled */ gen_exception_err(ctx, POWERPC_EXCP_ALIGN, POWERPC_EXCP_ALIGN_LE); return; } #endif } int ra, rd; TCGv EA; ... // rest of implementation P.S. I think there should be an alignment check after the EA is computed.