Re: [PATCH] target/openrisc: Set EPCR to next PC on FPE exceptions

[Richard Henderson <richard.henderson@linaro.org>]

On 7/29/23 14:08, Stafford Horne wrote:
> The architecture specification calls for the EPCR to be set to "Address
> of next not executed instruction" when there is a floating point
> exception (FPE).  This was not being done, so fix it by using the same
> method as syscall.  Note, this may need a lot more work if we start
> seeing floating point operations in delay slots which exceptions
> enabled.
> 
> Without this patch FPU exceptions will loop, as the exception hanlding
> will always return back to the failed floating point instruction.
> 
> This was not noticed in earlier testing because:
> 
>   1. The compiler usually generates code which clobbers the input operand
>      such as:
> 
>        lf.div.s r19,r17,r19
> 
>   2. The target will store the operation output before to the register
>      before handling the exception.  So an operation such as:
> 
>        float a = 100.0f;
>        float b = 0.0f;
>        float c = a / b;    /* lf.div.s r19,r17,r19 */
> 
>      Will first execute:
> 
>        100 / 0    -> Store inf to c (r19)
>                   -> triggering divide by zero exception
>                   -> handle and return
> 
>      Then it will exectute:
> 
>        100 / inf  -> Store 0 to c  (no exception)
> 
> To confirm the looping behavoid and the fix I used the following:
> 
>      float fpu_div(float a, float b) {
> 	float c;
> 	asm volatile("lf.div.s %0, %1, %2"
> 		      : "+r" (c)
> 		      : "r" (a), "r" (b));
> 	return c;
>      }
> 
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>   target/openrisc/interrupt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/openrisc/interrupt.c b/target/openrisc/interrupt.c
> index 3887812810..9b14b8a2c6 100644
> --- a/target/openrisc/interrupt.c
> +++ b/target/openrisc/interrupt.c
> @@ -34,7 +34,7 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
>       int exception = cs->exception_index;
>   
>       env->epcr = env->pc;
> -    if (exception == EXCP_SYSCALL) {
> +    if (exception == EXCP_SYSCALL || exception == EXCP_FPE) {
>           env->epcr += 4;
>       }
>       /* When we have an illegal instruction the error effective address

According to Table 6-3, when in a delay slot the EPCR should be the address of the jump, 
for both syscall and fpe.  This whole block should be moved down...

>     /* Set/clear dsx to indicate if we are in a delay slot exception.  */
>     if (env->dflag) {
>         env->dflag = 0;
>         env->sr |= SR_DSX;
>         env->epcr -= 4;
>     } else {
>         env->sr &= ~SR_DSX;
>     }

... into the else.

With that,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~