optimized negative check?

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

I'm doing a check for negative, like this:

		if (controlsignal < 0)					// Signal negative?
		{
			TurnOnRevMotor();					// Yes, reverse motor
			controlsignal	= ~controlsignal;	// Negate signal
			controlsignal	+= 1;
		}
		else
			TurnOnFwdMotor();

Problem is, the first line with the negative check seems to take about 1300 bytes! If it is replaced with

if (1)

the code size is 1300 bytes smaller. controlsignal is 64 bits wide, which might be part of the problem. How can I make a simple negative check?
Checking the most significant bit of controlsignal would be enough, but how is this done with 64 bits?

/Jakob Selbing

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

there is a problemin your logic...

May i suggest you take some representative values for control signal and put them through their paces in particular go manually through your algorithm when controlsignal equals 0, -1, +1, and respective positive and negative maxima.

dont forget if controlsignal = 0 then ~controlsignal = max positive value.

Good luck.

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

Negative numbers have the top bit set so the very top bit of the most significant byte of 'controlsignal' will be 1 for negative and 0 for positive. Assuming it's held little-endian then the MSB will be at byte address controlsignal+7 so I think something like the following may work:

unsigned char * p = &controlsignal + 7;

if (*p & x080) {
	// negative stuff
}
else {
	// positive stuff
}

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

ignoramus wrote:
there is a problemin your logic...

May i suggest you take some representative values for control signal and put them through their paces in particular go manually through your algorithm when controlsignal equals 0, -1, +1, and respective positive and negative maxima.

dont forget if controlsignal = 0 then ~controlsignal = max positive value.

Good luck.

The logic is simply to negate controlsignal if it's negative, i.e. get the absolute value of controlsignal. For 2's complement, negating is done by inverting all bits and adding 1. I haven't tested the code yet, but I don't see why it wouldn't work... ?

Clawson, thanks. I'll try it! I was trying to do this in assembler but I haven't fully understood how to mix assembler with C...

/Jakob Selbing

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

Hm, I forgot to mention, controlsignal is a local variable.

void MotorControlUpdate(void)
{
	int32_t	error;
	int64_t controlsignal;

and then

		if (controlsignal < 0)
			// negative
		else
			// positive

sort of.

GCC complains about

	unsigned char * temp = &controlsignal + 7;

with the message "warning: initialization from incompatible pointer type"
So how do I point to the MSB of controlsignal?

/Jakob Selbing

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

BTW, I was tryint to do it in assembler, like this:

		asm (	"mov __tmp_reg__, %H0" "\n\t"
				"andi __tmp_reg__, %1" "\n\t"
				: "=&r" (controlsignal) : "M" (0x80));
		if (bit_is_set(SREG, SREG_Z))					// Signal negative?
			TurnOnFwdMotor();							// No, MSb was 0
		else...

etc.

Is this the correct syntax? I cannot see what is actually produced by the compiler in the list file. And what about the SREG; is it possible that the compiler adds code between the "andi" and the if-statement, thereby corrupting the result?

/Jakob Selbing

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

jaksel wrote:
GCC complains about

	unsigned char * temp = &controlsignal + 7;

with the message "warning: initialization from incompatible pointer type"
So how do I point to the MSB of controlsignal?

The compiler just needs a bit of persuasion with a cast. Try something like:

unsigned char * temp = (unsigned char *)&controlsignal + 7;

Anyway it was a warning, not an error so the compiler would have generated the right code anyway. However it's good to clear warnings and the use of a cast like this tells the compiler that you really DO know what you are doing when you make a char pointer point at something that isn't a char.

Cliff

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

> The logic is simply to negate controlsignal if it's negative,
> i.e. get the absolute value of controlsignal. For 2's complement,
> negating is done by inverting all bits and adding 1.

Why would you want to assume a 2's complement implementation, instead
of simply writing:

controlsignal = -controlsignal;

? The world could be such an easy place occasionally...

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

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

dl8dtl wrote:
> The logic is simply to negate controlsignal if it's negative,
> i.e. get the absolute value of controlsignal. For 2's complement,
> negating is done by inverting all bits and adding 1.

Why would you want to assume a 2's complement implementation, instead
of simply writing:

controlsignal = -controlsignal;

? The world could be such an easy place occasionally...

That's a good question! :)

I just thought it would make the code smaller, which it apparently did (just checked it). I save 10 bytes with the 2's complement negation.
The problem with my code is that controlsignal is 64 bits wide, so even simple operations take a lot of space.

Edit: Of course, this will only work if controlsignal is in fact 2's complement. So, is it? How can I check?

/Jakob Selbing

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

jaksel wrote:
Of course, this will only work if controlsignal is in fact 2's complement. So, is it? How can I check?

Well apart from the fact that all signed compiler types will be 2's comp anyway I guess you could single step the generated code in the AVR Studio simulator and just watch what's happening to the bits and bytes to confirm your understanding.

(BTW I am quite intrigued to know why on earth the program needs 64 bit signed ints
anyway - do you really have a variable that counts up/down to into the region of
+/-922337203700000000 !?!?

Even if you wrote code so efficient that it incremented that variable once on every CPU cycle running at 8MHz it would take 1.1529E12 seconds to count from 0 to the +ve limit - that is 1.9e10 minutes or 320255973.7hours or 13343998.9 days or 36533.9 years !!!)

Cliff

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

clawson wrote:
jaksel wrote:
Of course, this will only work if controlsignal is in fact 2's complement. So, is it? How can I check?

Well apart from the fact that all signed compiler types will be 2's comp anyway I guess you could single step the generated code in the AVR Studio simulator and just watch what's happening to the bits and bytes to confirm your understanding.

(BTW I am quite intrigued to know why on earth the program needs 64 bit signed ints
anyway - do you really have a variable that counts up/down to into the region of
+/-922337203700000000 !?!?

Even if you wrote code so efficient that it incremented that variable once on every CPU cycle running at 8MHz it would take 1.1529E12 seconds to count from 0 to the +ve limit - that is 1.9e10 minutes or 320255973.7hours or 13343998.9 days or 36533.9 years !!!)

Cliff

Well, I know it's really way too much. The reason is I'm doing a servo controller. The position of the servo would need sth like 24 bits, so I use 32 bits. Then I calculate the control signal by multiplying this 32 bit value with gain constants, so strictly speaking I need more than 32 bits, so I use 64 bits. The control signal is then scaled down of course, to fit into the PWM register of the uC.

That leads me to another question: How do I declare variables of arbitrary bit length?
I simply tried e.g. uint24_t, but gcc complains...

/Jakob Selbing

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

> I just thought it would make the code smaller, which it apparently did (just checked it).

I agree that GCC could be smarter about that, and inline the operation.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

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

Well you can declare bit fields in a struct but at the end of the day everything is going to be a multiple of 8 bits and if you use an "odd" number then the compiler will just align accesses to the nearest 8 bit boundary anyway (or perhaps even more coarse depending on CPU architecture). On the whole standard compiler types are the x2 increments from 8 bits upwards - so 8, 16, 32, 64 - 24 doesn't fall into that series.

But 24 bits for servo position doesn't sound right. That means the motor can be positioned in 16,777,216 different positions? Assuming this is encoding a rotation somewhere between 0 and 360 degrees then each increment is 0.000021457 of a degree - I simply cannot believe that a motor armature can be positioned that accurately.

Cliff

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

clawson wrote:
Well you can declare bit fields in a struct but at the end of the day everything is going to be a multiple of 8 bits and if you use an "odd" number then the compiler will just align accesses to the nearest 8 bit boundary anyway (or perhaps even more coarse depending on CPU architecture). On the whole standard compiler types are the x2 increments from 8 bits upwards - so 8, 16, 32, 64 - 24 doesn't fall into that series.

But 24 bits for servo position doesn't sound right. That means the motor can be positioned in 16,777,216 different positions? Assuming this is encoding a rotation somewhere between 0 and 360 degrees then each increment is 0.000021457 of a degree - I simply cannot believe that a motor armature can be positioned that accurately.

Cliff


The servo has an "infinite" encoding, using digital magnetic sensors. It is designed to be able to move up to several 1000's of degrees in either direction, hence the width of the position.

/Jakob Selbing

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

Well even if it could move 2,048 one degree steps in either direction (so a total of about 11.3 revolutions) then it would still only have 4,096 distinct positions and that can be encoded in just 12 bits - not 24.

(and hence it'd easily fit into an 'int' and even if you wanted to do big maths on it it's unlikely you could spill out of a 'long' - the compiler even generates tons of code for handling long's, let alone int64's!)

Cliff

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

The specification actually says +/- 100,000 degrees at 0.5 deg resolution = 18.6 bits. Hence 32 bits for position. But part of the problem is that the gear ratio is variable, so I might end up with very high resolution.

/Jakob Selbing

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

So you just need 19 bit for the position.
If you take 32 bit for result there are 13 bit left for your multiplication with gain.

ciao, rg

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

Yep, that is true. I will probably change the code so it assumes only 19 bits are used. But another problem is that the specification says gear ratio can be anything between 1:1 and 10,000:1, so I have to allow very large gain for some ratios. 13 bits is hopefully sufficient.

/Jakob Selbing

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

Nope a factor of 10,000 would need 14 bits I think.

2^13 = 8192
2^14=16384

8192 < 10000 < 16384

Cliff

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

Yes, but I didn't say gain should be equal to gear ratio. What I meant is that gain must have many bits since gear ratio has many bits. But they don't have to be equally wide, it depends on how well the servo control works with low resolution.

Edit: Just realized, gear ratio might not matter at all for the control signal... so probably 32 bits is definitely enough for the control signal (19 bits for servo position + 13 bits for gain).

/Jakob Selbing