Compile questions - Routines within ISR

[basil4j 1]

Hi All,

 

I'm probably getting these due to my inexperience with C, but here goes...

Im getting the following remarks and errors during compilation.

 

"../main.c", line 321: remark #1538-D: (ULP 10.1) ISR i2c_send_complete calls function incr. Recommend moving function call away from ISR, or inlining the function, or using pragmas

 

I use this function to increment a pointer, and it is used mutiple times within the ISR so I dont really want to put it inline as it will take up a whole lot more code space...is this a code breaker? If it is, how do I use pragma?

 

Im getting this with other functions too, I just used this one as an example.

 

 

[enl 227]

This warning comes about because it is generally (but not always) bad practice to call a function from within an interrupt handler. Interrupt handlers should generally be as short as possible and as fast as possible. The function call overhead can slow things down, increasing response time to other interrupts, and add to the stack burden, which is a significant thing when you have limited memory or time critical response.

 

 

I would ask if you are sure that inlining it (or replacing the function with a #define macro) would increase code size. Incrementing a pointer isn't a big deal. Function calls take a bit of code space for setup and stack space for parameters. I presume that there is more than just incrementing a simple pointer, so.....

 

 

That said, there are times when it is perfectly acceptable to call a function within an interrupt handler. For example, if your interrupt is the only one active, and there is no chance of missing the next one because the interrupt rate is known to be low enough the routine will finish before the next interrupt, it is ok. Not best practice, but ok.

 

If other interrupts are active, and they can wait for this one to finish, and there is no chance of missing one, it is, again, ok, but not best practice.

 

Things to consider: A good compiler can determine the needed stack depth by tracking the call chain. This is not as easy if there are function calls in an interrupt, and may, in fact, be impossible if interrupts are re-enabled within the routine (not recommended on MSP430, IMHO). I don't know off hand if the compiler you are using does this-- I use CCS and have no idea if it does, as it has never been an issue for me. This is important in many cases, as if allows the compiler to manage RAM usage appropriately based on the context.

 

 

A better way to structure things, if you can, is have the interrupt routine do as little as possible, and handle everything else in your general code. The model that is commonly used is to have a main loop to do the work, and goes to sleep when the work is done. The interrupt does what it must, and resets the sleep on return (resets the low power mode bits on the MSP430), signalling what must be done for the main loop if needed. If this is not practical, and you can be sure that you won't lose due to memory or timing in the interrupt, go with it. Nothing says that the 'best' way is always the right way. I have shoved entirely too much into interrupt handlers at times, when it was the most practical solution for one reason or another.

 

 

The pragma referenced is likely the FUNC_CANNOT_INLINE pragma. This will tell the compiler that the function is not inlinable, and is will stop yelling at you about it, or DIAG_SUPRESS for the given message (before your function) paired with DIAG_DEFAULT (after your function)

[basil4j 1]

Thanks, I think that makes sense. The functions being called are what I would consider short, but having no previous experience with MSP, it would be good to have your opinion.

 

 

The following is used once within the interrupt, so I will move this inline.

int increment(int value, int max)
{
value++;
if (value == max) { value = 0; }
return value;
}

 

and used 9 times (i just realised this is a bad implementation of increment, so I will re-write and inline it too

 

void i2c_add_to_queue(char cmd)
{
i2c_queue[i2c_queue_write] = cmd;

i2c_queue_write++;
if(!(i2c_queue_write < QUEUE_LENGTH))
{
i2c_queue_write = 0;
}
}

and here is the largest one, used once in the interrupt but I am using it in other places also which is why I made it a function. The only reason this is so long is because its a giant switch/case. The actual number of tasks performed is quite small. 

void i2c_process_queue()
{
	switch (i2c_queue[i2c_queue_read])
	{
	case 1: //Accel data (write/read repeat start)
		i2c_send_buffer[0] = 0x32; // in this case, auto stop is after reading bytes. 0x32 is the start address
		i2c_start(0x1D, I2C_REPEATSTART, 6);
		break;
	case 2: //Start magneto sample (write)
		i2c_send_buffer[0] = 0x02;
		i2c_send_buffer[1] = 0x01;
		i2c_start(0x1E, I2C_WRITE, 2);
		break;
	case 3: //Get magneto data (write/read repeat start)
		i2c_send_buffer[0] = 0x03; // in this case, auto stop is after reading bytes. 0x03 is the start address
		i2c_start(0x1E, I2C_REPEATSTART, 6);
		break;
	case 4: //Baro - start temp sample (write)
		i2c_send_buffer[0] = 0x48; //4096 OSR
		i2c_start(0x77, I2C_WRITE, 1);
		break;
	case 5: //Baro - Get temp data (read)
		i2c_start(0x77, I2C_READ, 3);
		break;
	case 6: //Baro - Start pressure sample (write)
		i2c_send_buffer[0] = 0x58; //4096 OSR
		i2c_start(0x77, I2C_WRITE, 1);
		break;
	case 7: //Baro - Get temp data (read)
		i2c_start(0x77, I2C_READ, 3);
		break;
	case 8: // Baro - Ready ADC
		i2c_send_buffer[0] = 0x00;
		i2c_start(0x1E, I2C_WRITE, 1);
		break;
	default: //Empty, end of queue
		break;
	}
}

The interrupt with the longest code consists of 8 if statment's (worst case all 8 will be entered), and in each if statement i2c_add_to_queue will be called max 3 times. Is this too long?

[enl 227]

First two are definite inline. The calls may take as much or more space than the function will inlined and optimized. Certainly not a significant loss in the worst case.

 

The switch is a different matter. No issues using it in an interrupt (in my opinion) as switch is fast, but there is likely an interest in keeping it all in one place if it is called from elsewhere, and it is getting to a size where inlining is going to grow your code significantly.

 

My key concern here would be the call in the switch cases to i2c_start(). Does this take time? can it stall? Interrupts are blocked when this is called from within an ISR, and if that does anything significant, or has any interrupt dependent issues (like waiting for UART servicing, timer,  etc) you could have a bad day tracking down the stall.

 

If there are NO concerns, I'd tag this as not inlinable and see if the compiler is happy.

[basil4j 1]

Hi Enl,

 

Thanks for taking a look.

 

i2c_start simply sets up the USCI module depending on whether im writing, reading or writing with a repeat start before reading. Should be fairly fast as there is 1 short if statment and a handful of direct register access'.

void i2c_start(int addr, char cmd_type, int num_bytes)
{
	//Configure
	command_type = cmd_type;
	UCB0CTLW0 |= UCSWRST; // put eUSCI_B in reset state
	UCB0TBCNT = num_bytes; // automatic stop after x bytes
	UCB0CTLW0 &= ~UCSWRST; // eUSCI_B in operational state
	UCB0I2CSA = addr; // address of slave without r/w bit

	if (cmd_type == I2C_READ) // Read
	{
		UCB0CTLW0 &= !UCTR; // Put into receive mode
	} else {
		UCB0CTLW0 |= UCTR;  //transmitter mode
	}
	//Start
	UCB0CTLW0 |= UCTXSTT; // generate start condition
}

[enl 227]

I wouldn't worry about that, then, timewise, if there is no critical timing on the interrupt otherwise.

 

I'm not sure the pragma FUNC_CANNOT_INLINE will eliminate warning (and I am too lazy to set up a test right now... gotta get exams graded), but the warnings are just that: warnings. The intent is that you take a good hard look at what you are doing and determine if it is an issue.

 

Don;t ignire warnings, but a warning doesn't mean things won't work. Only that what you have is a place where a problem MAY occur.

[basil4j 1]

Ok neato Thanks!

[enl 227]

It is generally best, absent other information, determine whether inlining is worth it. The appropriate optimization settings will tell the compiler what you want, based on size vs. speed considerations, and it will do what is needed. Often, inlining leads to further optimization. Back in the day, it was common to write even fairly complex functions as #define macros for this reason.... a good compiler will optimize out the irrelevant, and optimize what isn't in the context of where it is used, rather than doing a best compromise on its own.

 

Long functions, with multiple calls, tend to do poorly, but even a moderately long function with only a couple calls may take both less space and time inline. The time and space overhead for a call  isn't large, but is also not zero. When you have five lines that compile to five single instructions (or even 4, or 3, if there are autoincrement opportunities), you are well below the overhead for the call. Your switch is probably better off not inline if called twice, almost certainly if called three times. The increment is certainly better off as inline. i2c_start() is questionable. It is short enough that inline may be break even or only a slight hit on space. I don't see anything that will shorten it, but it looks like about 8 or 10 instructions compiled, which is about what the call is with three parameters. try making it inline and look at the executable size. Even if it inlines to an instruction or two more than the call, you might break even or save by not having the function there as a standalone, or gain little enough to make it worthwhile. I don't know how hard you are pushing the memory, but a little experimentation canbe helpful in understanding what approaches work.

 

 

See: http://e2e.ti.com/support/development_tools/compiler/f/343/t/213752.aspx for a little info on optimization settings in CCS

[basil4j 1]

Thanks for all that! I will read the link you provided.

 

I dont think im pushing the memory that much. I dont know how to check how big the compiled program will be?

EDIT: Found out

 

Using 1950bytes of FRAM, and 952bytes RAM. So ive only used 12% of max FRAM which is good. 

I would say i've typed 60%-70% of the logic, but havent put any math into it yet (which will use MSP430_math library). Should have enough for that library though.

 

The big thing which gave me memory errors was a buffer which I want to make 1840 bytes...until I realised this would be in RAM which is only 1k Bytes haha

 

I haven't looked into it yet, but I guess there is someway I can keep this buffer in FRAM along with the program memory.

 

Ill point out I don't have my PCB made yet, just writing this program as much as I can while I wait Hope its not too buggy!