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

[Stafford Horne <shorne@gmail.com>]

On Thu, Aug 10, 2023 at 09:35:18AM +0300, Michael Tokarev wrote:
> 09.08.2023 23:34, Stafford Horne пишет:
> > 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
> > pattern as syscall.  Also, we move this logic down to be done for
> > instructions not in the delay slot as called for by the architecture
> > manual.
> > 
> > Without this patch FPU exceptions will loop, as the exception handling
> > 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 execute:
> > 
> >        100 / inf  -> Store 0 to c  (no exception)
> > 
> > To confirm the looping behavior 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;
> >      }
> 
> Is it a -stable material?  It applies cleanly to 8.0 and 7.2.
> Or maybe it is not needed on older versions, not being noticed before?

I would say no, it will work on 8.0 an 7.2 but this code path is not very useful
withouth the other 8.1 Floating Point Exception handling updates.

-Stafford