Re: [Qemu-devel] Re: Re: SH4: Implement FD bit

[Aurelien Jarno]

On Wed, Dec 10, 2008 at 02:04:27PM +0300, Vladimir Prus wrote:
> Aurelien Jarno wrote:
> 
> > On Tue, Dec 09, 2008 at 07:36:19PM +0300, Vladimir Prus wrote:
> >> Aurelien Jarno wrote:
> >> 
> >> >> 
> >> >> SH4 manual say that if a floating point instruction is executed while
> >> >> FD bit in the status register is 1, an exception should be raised. QEMU
> >> >> presently does not do that, so the kernel does not initialize FP state
> >> >> for any thread, nor does it save/restore FP state. The most apparent
> >> >> consequence is that while recent gcc/libc expect double-precision mode
> >> >> to be set by kernel, they run in single-precision mode, and all FP code
> >> >> produces wrong values.
> >> >> 
> >> >> This patch fixes this. It also fixes a couple of places where PC was
> >> >> not updated before handling an exception, although both those places
> >> >> deal with invalid instruction and don't lead to any user-visible bugs.
> >> >> 
> >> >> - Volodya
> >> > 
> >> > Thanks, applied.
> >> 
> >> Thanks, but it looks like one bit of the patch somehow did not
> >> make it into SVN. Specifically, this:
> >> 
> >> @@ -504,6 +523,13 @@ static void _decode_opc(DisasContext * ctx)
> >>               }
> >>         }
> >>  
> >> +    /* The 0xfffd instruction is underfined, so we don't want to
> >> +       raise fpu disable exception on it.  */
> >> +    if (((ctx->opcode & 0xf000) == 0xf000)
> >> +       && (ctx->opcode != 0xfffd))
> >> +      {
> >> +       CHECK_FPU_ENABLED
> >> +      }
> >> 
> >> Is present in my post, and is not present in:
> >> 
> >>         http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5937
> >> 
> >> Maybe some merge issue?
> > 
> > Yes I haven't seen that one hunk of the patch has been rejected, so I
> > missed this part. I have just tried to merge it by hand, but it is not
> > possible to apply it. It looks like the patch was not done against the
> > current SVN.
> > 
> > Care to resend it against the current SVN?
> 
> Attached is the remaining bit of the patch, against fresh SVN.

Actually I don't really like the idea of checking in the beginning of
the decode function for all instructions if they are a FP instruction,
and then checking if FPU is enabled. Given the number of instructions
(or group of instructions) concerned is not really big, I have committed
a patch that does the same by adding the check in each of the concerned
instruction.

-- 
  .''`.  Aurelien Jarno             | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   address@hidden         | address@hidden
   `-    people.debian.org/~aurel32 | www.aurel32.net