adding of "nop" fixes fixed-point multiply routine

Go To Last Post
6 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

After failing to compile avrfix into my project, I decided to simply copy-paste the bits I need from it into my "own" fixed point math routines. It didn't produce correct results, so I had to make some simple changes here and there. While doing so, I had some Serial.println() (using Arduino) code into the 15.16 multiply routine to see what was happening. After I got good multiplication results, I off course removed those lines. To my surprise, the results went bad again.

This is the test code:

_Accum a; // typedef signed long (15.16 fixed)
_Accum b;
a = ftok(1.5f); // float to fixed
b = mulk(a,a);  // multiply
Serial.println(ktof(a)); // check if ftok worked
Serial.println(ktof(b)); // check if mulk worked

It should produce (which it did with the Serial debugging I had in mulk()):

1.5
2.25

But without, it produced:

1.5
2.00

I simplified the fix by replacing the Serial debugging code with a "nop":

#define ASM_NOP() asm volatile ("nop" :: ) 

_Accum mulkD(_Accum x, _Accum y)
{

#define LO 0
#define HI 1

  unsigned short xs[2];
  unsigned short ys[2];
  int8_t positive = ((x < 0 && y < 0) || (y > 0 && x > 0)) ? 1 : 0;
  y = absk(y);
  *((_Accum*)xs) = absk(x);
  *((_Accum*)ys) = y;  
  x = sl(xs[HI])*y + sl(xs[LO])*ys[HI];
  //ASM_NOP();
  *((_Accum*)xs) = ul(xs[LO])*ul(ys[LO]);
  ASM_NOP();
 
  if(positive)
     return x + us(xs[HI]);
  else
     return -(x + us(xs[HI]));
#undef HI
#undef LO
}

The NOP has effect in 2 places, one is active in the code above, one commented out. After disassembly, the code for mulk is quite different, not just the addition of that NOP. Any ideas on what could be causing this?

I'm on avr-gcc 4.3.2 and my target is attiny84

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I'm on avr-gcc 4.3.2

Do you mean 4.7.2 or are you really stuck in a time warp?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Your type punning prodused undefined behavior.

There is no need for ftok, you can just assign float to accum, the compiler knows how to handle it.

As you are using avr-gcc 4.3.x with fixed-point support, this is an Atmel version. Some of these come with an incorrect implementation of 16*16 = 32 multiplication.

avrfreaks does not support Opera. Profile inactive.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm using Arduino so yes definetely stuck in a timewarp. It uses WinAVR 20081205, avr-gcc 4.3.2

So it doesn't have any support for fixed types, hence my own (well taken from avrfix) routines.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You should not do assignment like this

  unsigned short xs[2];
  *((_Accum*)xs) = 

Compiler does not understand that the assignment may change xs[1] and can use the old value instead.
Use union:

  union AA
  {
    unsigned short u[2];
    _Accum a;
  };
  AA xs;
  xs.a =
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks, not only did it fix the problem, it also makes the whole thing much easier to grasp :)