Re: [Qemu-devel] Floating point unit bugs

[Aurelien Jarno]

On 2017-05-08 18:36, G 3 wrote:
> 
> On May 8, 2017, at 6:13 PM, Aurelien Jarno wrote:
> 
> > On 2017-05-07 17:48, G 3 wrote:
> > > I made a diagnostic program for the floating point unit. It will test
> > > various PowerPC floating point instructions for compatibility with
> > > the
> > > PowerPC G3 processor. It was tested on a PowerPC G3 and G5 system.
> > > The
> > > results of the program in qemu-system-ppc were pretty bad. About
> > > every
> > > instruction tested is not implemented correctly.
> > > 
> > > Here is the download link to the program: 
> > > http://www.mediafire.com/file/6j9tqubvk73lkw1/floating_point_test_program.zip
> > 
> > Some comments on the code.
> > > 
> > >     /* Check if everything is alright */
> > >     uint32_t fpscr;
> > >     asm volatile("mffs f0");
> > >     asm volatile("stfd f0, 40(r1)");
> > >     asm volatile("lwz %0, 44(r1)" : "=r"(fpscr));
> > >     if (fpscr != 0) {
> > >         printf("Warning: fpscr not equal to zero: 0x%x\n", fpscr);
> > >     }
> > 
> > This is overly complicated and just doesn't compile with recent GCC
> > versions.
> 
> Which version of GCC had the problem? GCC 5.2 and GCC 3.3 seems to work
> fine. GCC 4.0 did not work. Could you send me the error message?

I tried with GCC 4.9. Actually the error message is coming from
binutils:

| main.c:34:5: warning: missing braces around initializer [-Wmissing-braces]
|      "FX", "Floating-point exception summary",
|      ^
| main.c:34:5: warning: (near initialization for 'finfo[0]') [-Wmissing-braces]
| main.c: In function 'print_fpscr_settings':
| main.c:73:26: warning: suggest parentheses around comparison in operand of 
'&' [-Wparentheses]
|          if ((fpscr >> i) & 0x1 == 1) {
|                           ^
| main.c: In function 'test_failed':
| main.c:146:5: warning: format '%llx' expects argument of type 'long long 
unsigned int', but argument 2 has type 'uint32_t' [-Wformat=]
|      printf("  actual answer: 0x%" PRIx64 "\n", actual_fpscr);
|      ^
| /tmp/cctHPRx4.s: Assembler messages:
| /tmp/cctHPRx4.s:315: Error: unsupported relocation against f0
| /tmp/cctHPRx4.s:318: Error: unsupported relocation against f0
| /tmp/cctHPRx4.s:318: Error: unsupported relocation against r1
| /tmp/cctHPRx4.s:321: Error: unsupported relocation against r1
| /tmp/cctHPRx4.s:438: Error: unsupported relocation against f0
| /tmp/cctHPRx4.s:441: Error: unsupported relocation against f0
| /tmp/cctHPRx4.s:441: Error: unsupported relocation against r1
| /tmp/cctHPRx4.s:444: Error: unsupported relocation against r1

> > What about something like:
> > 
> >     Converter c;
> >     asm volatile("mffs %0" : "=f"(c.d));
> >     fpscr = (uint32_t)c.i;
> 
> This way does work also.
> 
> > 
> > 
> > > /*
> > >  * The action to take if a test fails
> > >  * Input one: message string
> > >  * Input two: actual fpscr value
> > >  * Input three: expected fpscr value
> > >  * Input four: actual answer
> > >  * Input five: expected answer
> > >  */
> > >  void test_failed(const char *message, uint32_t actual_fpscr,
> > > uint32_t
> > > expected_fpscr,
> > >                   uint64_t actual_answer, uint64_t expected_answer)
> > >  {
> > >     printf("%s\n", message);
> > >     printf("expected answer: 0x%" PRIx64 "\n", expected_answer);
> > >     printf("  actual answer: 0x%" PRIx64 "\n", actual_fpscr);
> > 
> > This is wrong. It should be actual_answer instead of actual_fpscr. That
> > is why all the instructions seems totally wrongly implemented.
> 
> Thanks for catching this error. Actually this would only effect the "actual
> answer:" output field. The comparison between expected_answer and
> actual_answer in each individual test is still valid.

Indeed, but I guess it gives "better" results than what it looks when
looking at your mail where the values are totally wrong.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net