From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39385) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXVky-0006wn-As for qemu-devel@nongnu.org; Thu, 03 Sep 2015 10:44:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZXVku-0003VB-9f for qemu-devel@nongnu.org; Thu, 03 Sep 2015 10:44:52 -0400 Received: from mail-qk0-x22c.google.com ([2607:f8b0:400d:c09::22c]:34032) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXVku-0003V3-5k for qemu-devel@nongnu.org; Thu, 03 Sep 2015 10:44:48 -0400 Received: by qkfq186 with SMTP id q186so21488605qkf.1 for ; Thu, 03 Sep 2015 07:44:47 -0700 (PDT) Sender: Richard Henderson References: <1441239463-18981-1-git-send-email-rth@twiddle.net> <1441239463-18981-3-git-send-email-rth@twiddle.net> <55E85633.3070909@mail.uni-paderborn.de> From: Richard Henderson Message-ID: <55E85CDC.7040003@twiddle.net> Date: Thu, 3 Sep 2015 07:44:44 -0700 MIME-Version: 1.0 In-Reply-To: <55E85633.3070909@mail.uni-paderborn.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 02/17] target-openrisc: Streamline arithmetic and OVE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bastian Koppelmann , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, proljc@gmail.com On 09/03/2015 07:16 AM, Bastian Koppelmann wrote: > > > On 09/03/2015 02:17 AM, Richard Henderson wrote: >> + >> +void HELPER(ove)(CPUOpenRISCState *env, target_ulong test) >> +{ >> + if (unlikely(test) && (env->sr & SR_OVE)) { >> + OpenRISCCPU *cpu = openrisc_env_get_cpu(env); >> + CPUState *cs = CPU(cpu); >> + >> + cs->exception_index = EXCP_RANGE; >> + cpu_restore_state(cs, GETPC()); >> + cpu_loop_exit(cs); >> + } >> +} > > You forgot to check the AECR register, whether the exception really occurs. The current state of the port suggests it's written to a (pre?) 1.0 specification, prior to the AECR register being invented. Let's not try to introduce new features just yet. >> +static void gen_ove_cy(DisasContext *dc, TCGv cy) >> +{ >> + gen_helper_ove(cpu_env, cy); >> +} >> + >> +static void gen_ove_ov(DisasContext *dc, TCGv ov) >> +{ >> + gen_helper_ove(cpu_env, ov); >> +} >> + > > Do we really need two functions here? They differ only by the name of the > second argument. We should directly use gen_helper_ove (). This is prep work for patch 7/17. > I do wonder, if we should use TCG globals for sr_cy and sr_ov, as you > recommended for the TriCore target. It would not change the helper in case of > no ovf/carry, but simplify addc. And we would not need two deposits for every > add/sub. That's patch 7/17. ;-) > >> - if (ra != 0 && rb != 0) { >> - gen_helper_mul32(cpu_R[rd], cpu_env, cpu_R[ra], cpu_R[rb]); >> - } else { >> - tcg_gen_movi_tl(cpu_R[rd], 0x0); >> - } >> + gen_mul(dc, cpu_R[rd], cpu_R[ra], cpu_R[rb]); > > What happened to this special case here? r0 should always hold zero, so we can > keep this optimization here, or at least move it to gen_mul(). R0 is not an *architectural* zero. It's a software convention. The spec is fairly clear on this point. I agree that there ought to be some special-casing of the software convention, but that should require a TB flag to verify the convention is followed. But even then I would not bother retaining the special case here in multiply. I would only use it in the "move" and constant formation types of pseudo instructions (or, ori, etc). r~