AVR Freaks

I suppose another approach might be:

typedef struct {
    volatile uint8_t * port;
    uint8_t pin;
} port_pin_t;

port_pin_t pinarray[] = {
    { &PORTB, (1 << 3) },
    { &PORTD, (1 << 2) },
    { &PORTA, (1 << 5) },
    { &PORTC, (1 << 7) },
    etc.
}

static inline set_pin(int n) {
    pinarray[n].port |= pinarray[n].pin;
}

static inline clr_pin(int n) {
    pinarray[n].port &= ~pinarray[n].pin;
}

then later:

set_pin(5);
clr_pin(11);

In all cases there is going to be some overhead for trying to give the port/pin groups a virtual ID (0.. 11). Far better is just to say:

PORTB &= ~(1 << 3);

exactly where you need that bit in that port clear. If you want to "dress it up":

#define WIDGET_VALVE_PORT PORTB
#define WIDGET_VALVE_PIN (1 << 3)

WIDGET_VALVE_PORT &= ~WIDGET_VALVE_PIN;

it's true that this does not lend itself to:

for (int i = 0; i < 12; i++) {
    clr_valve(i);
}

which is what making an "array" out of the combinations - but exactly how important and required is that anyway?

Most definitely more - the AVR is a RISC processor so each opcode can only achieve one small step at a time. Not enough detail - I've no idea what you are trying to achieve but it does seem pretty clear that 8 steps of that are setting all of bits 0..7 in PORTA. If speed is of the essence why would you encode the pointless loop? Why wouldn't you simply say:

PORTB = 0xFF;

That is probably one LDI and one STS so about 3..4 cycles to set all 8 bits at once.

 

Seems you are focusing too much on making this 0..11 pin numbering thing that you have lost sight of what it is you are actually trying to do! if that entire loop is about setting 8 bits in one port and 2..3 in another then two statements:

PORTA = 0xFF;
PORTB = 0x07;

(or whatever) achieve this without a lot of pointless pin mapping.

 

Is this all because one day the thing on PORTB bit 3 might move to PORTC bit 5 or something? Does hardware really get up and shuffle itself about during the night then?

In your post #8 you are shifting bits in software, which is a relatively expensive operation for an AVR.

 

The execution speed of a switch is pretty much a guess without trying it out. The compiler is free to do a whole lot of optimisations here.

Sometimes a simple lookup table may be used.

If numbers are consecutive, then an offset calculation with a jump table may be used.

Sometimes a binary tree search may be implemented in the background.

The array method in clawson's #7 is probably the fastest method.

Once the array index " n " is known, the AVR only has to fetch some values form memory and do the |= or &= to set a bit of an IO register.

Normally an " |= " or an " &= ~ " is translated to a single sbi or cbi instruction, but the compiler probably can not do that here because of the loading of values from an array.

 

The only way to do it in C which might be faster is to make functions for each bit set or clear operation, make an array of these functions and use a function pointer to execute the right function from the array.

 

But it is all too much work to bother about.

You need to sit back, scratch yourself behind the ear, and think about what kind of speed you need. The switch() method probably takes a few micro seconds. Is that good enough for you? How much performance do you need? 1us, 10us, Is 100us acceptable? The Xmega's are also quite a bit faster than the AVR's i'm used to.

The software is very likely already faster than the time to turn on a power transistor, the time needed for a magnetic field in the valves to build up, and the time needed for mechanical things to move in your valves / relays, whatever. Those combined can very easily be several milli seconds.

 

I also thing you have made the beginner mistake of assuming that the amount of text in a program is correlated to the execution speed of a compiled program.

There is no straightforward correlation. And on top of that the compiler has a big bag full of tricks to optimize code and those tricks may or may not kick in depending on somtimes obfuscated details.

If you want to go down to that level, you should study the .LSS output of the compiler. GCC uses assembly as an intermediate step to create a binary and this listing file has the assembly output of the compiler combined with the C source code as comment. So you can see from each line of C source code what machine code is acutally generated.

Hint:

The shift in your example #8 has a variable as rvalue, which means the shift has to be done at execution time.

The shift in other examples has a constant  as rvalue, which means the shift is calculated by te preprocessor and loaded as a constant in the final binary.

 

https://www.xkcd.com/1691/

OK, so I had to try this for real...

/*
 * test1.c
 *
 * Created: 24/09/2018 13:23:01
 * Author : uid23021
 */ 
#include <avr/io.h>

typedef enum {
	HYDRAULIC = 0,
	PNEUMATIC,
	WIDGET,
	SPLINE_SHAFT
} valve_e;

typedef struct {
	volatile PORT_t * port;
	uint8_t pin;
} port_pin_t;

port_pin_t pinarray[] = {
	{ &PORTB, (1 << 3) },
	{ &PORTD, (1 << 2) },
	{ &PORTA, (1 << 5) },
	{ &PORTC, (1 << 7) }
};

static inline void set(int n) {
	pinarray[n].port->OUTSET = pinarray[n].pin;
}

static inline void clr(int n) {
	pinarray[n].port->OUTCLR = pinarray[n].pin;
}

int main(void)
{
	set(HYDRAULIC);
	clr(PNEUMATIC);
	clr(WIDGET);
	set(SPLINE_SHAFT);
}

The first invocation "set(HYDRAULIC)" yields:

	pinarray[n].port->OUTSET = pinarray[n].pin;
 23c:	e0 e0       	ldi	r30, 0x00	; 0
 23e:	f0 e2       	ldi	r31, 0x20	; 32
 240:	a0 81       	ld	r26, Z
 242:	b1 81       	ldd	r27, Z+1	; 0x01
 244:	82 81       	ldd	r24, Z+2	; 0x02
 246:	15 96       	adiw	r26, 0x05	; 5
 248:	8c 93       	st	X, r24

The array is at 0x2000 in RAM. That loads the pin number into R24 and the pointer to the PORT_t into R27:R26 (aka X). It then adds an offset of 5 because OUTSET is the 6th element in the port structure. Then it writes the pin mask to the register. The next code "clr(PNEUMATIC)" yields:

    pinarray[n].port->OUTCLR = pinarray[n].pin;
 24a:	a3 81       	ldd	r26, Z+3	; 0x03
 24c:	b4 81       	ldd	r27, Z+4	; 0x04
 24e:	85 81       	ldd	r24, Z+5	; 0x05
 250:	16 96       	adiw	r26, 0x06	; 6
 252:	8c 93       	st	X, r24

this is relying on the fact that Z is already set so now it loads the pinarray[1] details with pin mask in R24 and the PORT_t address in X. This time the OUTCLR is at offset 6 in the port structure.

 

Just to compare, this code:

	PORTB.OUTSET = (1 << 3);
	PORTD.OUTCLR = (1 << 2);
	PORTA.OUTCLR = (1 << 5);
	PORTC.OUTSET = (1 << 7);

which is the most direct way to set and clear the bits yields:

	PORTB.OUTSET = (1 << 3);
 268:	88 e0       	ldi	r24, 0x08	; 8
 26a:	80 93 25 06 	sts	0x0625, r24	; 0x800625 <__TEXT_REGION_LENGTH__+0x700625>
	PORTD.OUTCLR = (1 << 2);
 26e:	84 e0       	ldi	r24, 0x04	; 4
 270:	80 93 66 06 	sts	0x0666, r24	; 0x800666 <__TEXT_REGION_LENGTH__+0x700666>
	PORTA.OUTCLR = (1 << 5);
 274:	80 e2       	ldi	r24, 0x20	; 32
 276:	80 93 06 06 	sts	0x0606, r24	; 0x800606 <__TEXT_REGION_LENGTH__+0x700606>
	PORTC.OUTSET = (1 << 7);
 27a:	80 e8       	ldi	r24, 0x80	; 128
 27c:	80 93 45 06 	sts	0x0645, r24	; 0x800645 <__TEXT_REGION_LENGTH__+0x700645>

So there is a "cost" for doing this indirectly using the array access thing simply so you can enumerate the pins. Of course what this latter code does not cater for is:

#define NUM_ELEMENTS(x) sizeof(x)/sizeof(x[0])
	for(int i = 0; i < NUM_ELEMENTS(pinarray); i++) {
		set(i);
	}

where that code yields:

	for(int i = 0; i < NUM_ELEMENTS(pinarray); i++) {
 290:	80 e2       	ldi	r24, 0x20	; 32
 292:	ec 30       	cpi	r30, 0x0C	; 12
 294:	f8 07       	cpc	r31, r24
 296:	39 f0       	breq	.+14     	; 0x2a6 <main+0x6a>
	{ &PORTA, (1 << 5) },
	{ &PORTC, (1 << 7) }
};

static inline void set(int n) {
	pinarray[n].port->OUTSET = pinarray[n].pin;
 298:	a0 81       	ld	r26, Z
 29a:	b1 81       	ldd	r27, Z+1	; 0x01
 29c:	82 81       	ldd	r24, Z+2	; 0x02
 29e:	15 96       	adiw	r26, 0x05	; 5
 2a0:	8c 93       	st	X, r24
 2a2:	33 96       	adiw	r30, 0x03	; 3
 2a4:	f5 cf       	rjmp	.-22     	; 0x290 <main+0x54>

 

Yes, What I am showing is a way to make an arbitrary list of Ports and Pins. So my list above sets pinarry[0] to PORTB.3, [1] to PORTD.2, [2] to PORTA.5, [3] to PORTC.7

 

It could just as easily have been:

port_pin_t pinarray[] = {
	{ &PORTB, (1 << 0) },
	{ &PORTB, (1 << 1) },
	{ &PORTB, (1 << 2) },
	{ &PORTB, (1 << 5) }
};

or something like that where it's almost all consecutive pins in PORTB with just PORTB.5 being the out of sequence exception. That is PB0, PB1, PB2 and PB5.