* [PATCH 0/2] arm: Implement M-profile trapping on division by zero
@ 2021-07-30 15:16 Peter Maydell
2021-07-30 15:16 ` [PATCH 1/2] target/arm: Re-indent sdiv and udiv helpers Peter Maydell
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Peter Maydell @ 2021-07-30 15:16 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Unlike A-profile, for M-profile the UDIV and SDIV insns can be
configured to raise an exception on division by zero, using the CCR
DIV_0_TRP bit. This patchset implements that missing functionality
by having the udiv and sdiv helpers raise an exception if needed.
Some questions:
Is it worth allowing A-profile to retain the mildly better codegen it
gets from not having to pass in 'env' and marking the helper as
no-side-effects (ie having M-specific udiv/sdiv helpers) ?
Is it worth inlining either udiv or sdiv for the A-profile case?
udiv can be done with movcond/movcond/divu, something like:
/* t1 = (t2 == 0) ? 0 : t1; t2 = (t2 == 0) ? 1 : t2 */
tcg_gen_movcond_i32(TCG_COND_EQ, t1, t2, tcg_constant_i32(0),
tcg_constant_i32(0), t1);
tcg_gen_movcond_i32(TCG_COND_EQ, t2, t2, tcg_constant_i32(0),
tcg_constant_i32(1), t2);
/* Either t1 / t2; or 0 / 1 to give 0 for division-by-zero */
tcg_gen_divu_i32(t1, t1, t2);
sdiv is more painful because it needs to check for both x/0 and
INTMIN/-1 cases. Some other targets choose to generate inline TCG
ops for it, though.
Side note, I don't understand the x86-64 codegen for the above
sketch of an inline udiv. When I try it the TCG ops are
mov_i32 tmp3,r2
mov_i32 tmp6,r3
movcond_i32 tmp3,tmp6,$0x0,$0x0,tmp3,eq
movcond_i32 tmp6,tmp6,$0x0,$0x1,tmp6,eq
mov_i32 tmp7,$0x0
divu2_i32 tmp3,tmp7,tmp3,tmp7,tmp6
mov_i32 r3,tmp3
but the x86 code is
0x7f5f1807dc0c: 45 33 f6 xorl %r14d, %r14d
0x7f5f1807dc0f: 45 85 ed testl %r13d, %r13d
0x7f5f1807dc12: 45 0f 44 e6 cmovel %r14d, %r12d
0x7f5f1807dc16: 41 bf 01 00 00 00 movl $1, %r15d
0x7f5f1807dc1c: 45 3b ee cmpl %r14d, %r13d
0x7f5f1807dc1f: 45 0f 44 ef cmovel %r15d, %r13d
0x7f5f1807dc23: 41 8b c4 movl %r12d, %eax
0x7f5f1807dc26: 41 8b d6 movl %r14d, %edx
0x7f5f1807dc29: 41 f7 f5 divl %r13d
where the comparison for the first cmovel is 'testl %r13d, %r13d",
but the second comparison is 'cmpl %r14d, %r13d'. That's the same
effect (given r14 is 0) but I don't understand why the backend has
chosen to generate different code for the two cases. (Ideally of
course it would notice that it already had generated the condition
check and not repeat it.)
thanks
-- PMM
Peter Maydell (2):
target/arm: Re-indent sdiv and udiv helpers
target/arm: Implement M-profile trapping on division by zero
target/arm/cpu.h | 1 +
target/arm/helper.h | 4 ++--
target/arm/helper.c | 34 ++++++++++++++++++++++++++--------
target/arm/m_helper.c | 4 ++++
target/arm/translate.c | 4 ++--
5 files changed, 35 insertions(+), 12 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] target/arm: Re-indent sdiv and udiv helpers 2021-07-30 15:16 [PATCH 0/2] arm: Implement M-profile trapping on division by zero Peter Maydell @ 2021-07-30 15:16 ` Peter Maydell 2021-08-02 22:23 ` Richard Henderson 2021-07-30 15:16 ` [PATCH 2/2] target/arm: Implement M-profile trapping on division by zero Peter Maydell 2021-08-02 22:23 ` [PATCH 0/2] arm: " Richard Henderson 2 siblings, 1 reply; 6+ messages in thread From: Peter Maydell @ 2021-07-30 15:16 UTC (permalink / raw) To: qemu-arm, qemu-devel We're about to make a code change to the sdiv and udiv helper functions, so first fix their indentation and coding style. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index 155d8bf2399..8e9c2a2cf8c 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -9355,17 +9355,20 @@ uint32_t HELPER(uxtb16)(uint32_t x) int32_t HELPER(sdiv)(int32_t num, int32_t den) { - if (den == 0) - return 0; - if (num == INT_MIN && den == -1) - return INT_MIN; + if (den == 0) { + return 0; + } + if (num == INT_MIN && den == -1) { + return INT_MIN; + } return num / den; } uint32_t HELPER(udiv)(uint32_t num, uint32_t den) { - if (den == 0) - return 0; + if (den == 0) { + return 0; + } return num / den; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] target/arm: Re-indent sdiv and udiv helpers 2021-07-30 15:16 ` [PATCH 1/2] target/arm: Re-indent sdiv and udiv helpers Peter Maydell @ 2021-08-02 22:23 ` Richard Henderson 0 siblings, 0 replies; 6+ messages in thread From: Richard Henderson @ 2021-08-02 22:23 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel On 7/30/21 5:16 AM, Peter Maydell wrote: > We're about to make a code change to the sdiv and udiv helper > functions, so first fix their indentation and coding style. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > target/arm/helper.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] target/arm: Implement M-profile trapping on division by zero 2021-07-30 15:16 [PATCH 0/2] arm: Implement M-profile trapping on division by zero Peter Maydell 2021-07-30 15:16 ` [PATCH 1/2] target/arm: Re-indent sdiv and udiv helpers Peter Maydell @ 2021-07-30 15:16 ` Peter Maydell 2021-08-02 22:23 ` Richard Henderson 2021-08-02 22:23 ` [PATCH 0/2] arm: " Richard Henderson 2 siblings, 1 reply; 6+ messages in thread From: Peter Maydell @ 2021-07-30 15:16 UTC (permalink / raw) To: qemu-arm, qemu-devel Unlike A-profile, for M-profile the UDIV and SDIV insns can be configured to raise an exception on division by zero, using the CCR DIV_0_TRP bit. Implement support for setting this bit by making the helper functions raise the appropriate exception. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/cpu.h | 1 + target/arm/helper.h | 4 ++-- target/arm/helper.c | 19 +++++++++++++++++-- target/arm/m_helper.c | 4 ++++ target/arm/translate.c | 4 ++-- 5 files changed, 26 insertions(+), 6 deletions(-) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 9f0a5f84d50..5cf8996ae3c 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -54,6 +54,7 @@ #define EXCP_LAZYFP 20 /* v7M fault during lazy FP stacking */ #define EXCP_LSERR 21 /* v8M LSERR SecureFault */ #define EXCP_UNALIGNED 22 /* v7M UNALIGNED UsageFault */ +#define EXCP_DIVBYZERO 23 /* v7M DIVBYZERO UsageFault */ /* NB: add new EXCP_ defines to the array in arm_log_exception() too */ #define ARMV7M_EXCP_RESET 1 diff --git a/target/arm/helper.h b/target/arm/helper.h index 248569b0cd8..aee8f0019b4 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -6,8 +6,8 @@ DEF_HELPER_3(add_saturate, i32, env, i32, i32) DEF_HELPER_3(sub_saturate, i32, env, i32, i32) DEF_HELPER_3(add_usaturate, i32, env, i32, i32) DEF_HELPER_3(sub_usaturate, i32, env, i32, i32) -DEF_HELPER_FLAGS_2(sdiv, TCG_CALL_NO_RWG_SE, s32, s32, s32) -DEF_HELPER_FLAGS_2(udiv, TCG_CALL_NO_RWG_SE, i32, i32, i32) +DEF_HELPER_FLAGS_3(sdiv, TCG_CALL_NO_RWG, s32, env, s32, s32) +DEF_HELPER_FLAGS_3(udiv, TCG_CALL_NO_RWG, i32, env, i32, i32) DEF_HELPER_FLAGS_1(rbit, TCG_CALL_NO_RWG_SE, i32, i32) #define PAS_OP(pfx) \ diff --git a/target/arm/helper.c b/target/arm/helper.c index 8e9c2a2cf8c..56c520cf8e9 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -9345,6 +9345,18 @@ uint32_t HELPER(sxtb16)(uint32_t x) return res; } +static void handle_possible_div0_trap(CPUARMState *env, uintptr_t ra) +{ + /* + * Take a division-by-zero exception if necessary; otherwise return + * to get the usual non-trapping division behaviour (result of 0) + */ + if (arm_feature(env, ARM_FEATURE_M) + && (env->v7m.ccr[env->v7m.secure] & R_V7M_CCR_DIV_0_TRP_MASK)) { + raise_exception_ra(env, EXCP_DIVBYZERO, 0, 1, ra); + } +} + uint32_t HELPER(uxtb16)(uint32_t x) { uint32_t res; @@ -9353,9 +9365,10 @@ uint32_t HELPER(uxtb16)(uint32_t x) return res; } -int32_t HELPER(sdiv)(int32_t num, int32_t den) +int32_t HELPER(sdiv)(CPUARMState *env, int32_t num, int32_t den) { if (den == 0) { + handle_possible_div0_trap(env, GETPC()); return 0; } if (num == INT_MIN && den == -1) { @@ -9364,9 +9377,10 @@ int32_t HELPER(sdiv)(int32_t num, int32_t den) return num / den; } -uint32_t HELPER(udiv)(uint32_t num, uint32_t den) +uint32_t HELPER(udiv)(CPUARMState *env, uint32_t num, uint32_t den) { if (den == 0) { + handle_possible_div0_trap(env, GETPC()); return 0; } return num / den; @@ -9567,6 +9581,7 @@ void arm_log_exception(int idx) [EXCP_LAZYFP] = "v7M exception during lazy FP stacking", [EXCP_LSERR] = "v8M LSERR UsageFault", [EXCP_UNALIGNED] = "v7M UNALIGNED UsageFault", + [EXCP_DIVBYZERO] = "v7M DIVBYZERO UsageFault", }; if (idx >= 0 && idx < ARRAY_SIZE(excnames)) { diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c index 20761c94877..47903b3dc35 100644 --- a/target/arm/m_helper.c +++ b/target/arm/m_helper.c @@ -2252,6 +2252,10 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, env->v7m.secure); env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_UNALIGNED_MASK; break; + case EXCP_DIVBYZERO: + armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, env->v7m.secure); + env->v7m.cfsr[env->v7m.secure] |= R_V7M_CFSR_DIVBYZERO_MASK; + break; case EXCP_SWI: /* The PC already points to the next instruction. */ armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SVC, env->v7m.secure); diff --git a/target/arm/translate.c b/target/arm/translate.c index 80c282669f0..28eabeb2323 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -7992,9 +7992,9 @@ static bool op_div(DisasContext *s, arg_rrr *a, bool u) t1 = load_reg(s, a->rn); t2 = load_reg(s, a->rm); if (u) { - gen_helper_udiv(t1, t1, t2); + gen_helper_udiv(t1, cpu_env, t1, t2); } else { - gen_helper_sdiv(t1, t1, t2); + gen_helper_sdiv(t1, cpu_env, t1, t2); } tcg_temp_free_i32(t2); store_reg(s, a->rd, t1); -- 2.20.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] target/arm: Implement M-profile trapping on division by zero 2021-07-30 15:16 ` [PATCH 2/2] target/arm: Implement M-profile trapping on division by zero Peter Maydell @ 2021-08-02 22:23 ` Richard Henderson 0 siblings, 0 replies; 6+ messages in thread From: Richard Henderson @ 2021-08-02 22:23 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel On 7/30/21 5:16 AM, Peter Maydell wrote: > Unlike A-profile, for M-profile the UDIV and SDIV insns can be > configured to raise an exception on division by zero, using the CCR > DIV_0_TRP bit. > > Implement support for setting this bit by making the helper functions > raise the appropriate exception. > > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > target/arm/cpu.h | 1 + > target/arm/helper.h | 4 ++-- > target/arm/helper.c | 19 +++++++++++++++++-- > target/arm/m_helper.c | 4 ++++ > target/arm/translate.c | 4 ++-- > 5 files changed, 26 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] arm: Implement M-profile trapping on division by zero 2021-07-30 15:16 [PATCH 0/2] arm: Implement M-profile trapping on division by zero Peter Maydell 2021-07-30 15:16 ` [PATCH 1/2] target/arm: Re-indent sdiv and udiv helpers Peter Maydell 2021-07-30 15:16 ` [PATCH 2/2] target/arm: Implement M-profile trapping on division by zero Peter Maydell @ 2021-08-02 22:23 ` Richard Henderson 2 siblings, 0 replies; 6+ messages in thread From: Richard Henderson @ 2021-08-02 22:23 UTC (permalink / raw) To: Peter Maydell, qemu-arm, qemu-devel On 7/30/21 5:16 AM, Peter Maydell wrote: > Unlike A-profile, for M-profile the UDIV and SDIV insns can be > configured to raise an exception on division by zero, using the CCR > DIV_0_TRP bit. This patchset implements that missing functionality > by having the udiv and sdiv helpers raise an exception if needed. > > Some questions: > > Is it worth allowing A-profile to retain the mildly better codegen it > gets from not having to pass in 'env' and marking the helper as > no-side-effects (ie having M-specific udiv/sdiv helpers) ? Probably not. > Is it worth inlining either udiv or sdiv for the A-profile case? Probably not. > mov_i32 tmp3,r2 > mov_i32 tmp6,r3 > movcond_i32 tmp3,tmp6,$0x0,$0x0,tmp3,eq > movcond_i32 tmp6,tmp6,$0x0,$0x1,tmp6,eq > mov_i32 tmp7,$0x0 > divu2_i32 tmp3,tmp7,tmp3,tmp7,tmp6 > mov_i32 r3,tmp3 > > but the x86 code is > 0x7f5f1807dc0c: 45 33 f6 xorl %r14d, %r14d > 0x7f5f1807dc0f: 45 85 ed testl %r13d, %r13d > 0x7f5f1807dc12: 45 0f 44 e6 cmovel %r14d, %r12d At the start of the first movcond, $0x0 is not allocated to a register, and the constraints allow a constant for argument 3. Then, constraints do not allow a constant for argument 4 so we load $0x0 into a register. > 0x7f5f1807dc16: 41 bf 01 00 00 00 movl $1, %r15d > 0x7f5f1807dc1c: 45 3b ee cmpl %r14d, %r13d > 0x7f5f1807dc1f: 45 0f 44 ef cmovel %r15d, %r13d At the start of the second movcond, $0x0 is loaded into a register, so we use it. > (Ideally of > course it would notice that it already had generated the condition > check and not repeat it.) Yep. r~ ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-02 22:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-30 15:16 [PATCH 0/2] arm: Implement M-profile trapping on division by zero Peter Maydell 2021-07-30 15:16 ` [PATCH 1/2] target/arm: Re-indent sdiv and udiv helpers Peter Maydell 2021-08-02 22:23 ` Richard Henderson 2021-07-30 15:16 ` [PATCH 2/2] target/arm: Implement M-profile trapping on division by zero Peter Maydell 2021-08-02 22:23 ` Richard Henderson 2021-08-02 22:23 ` [PATCH 0/2] arm: " Richard Henderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).