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~
