AVR Freaks

I would head in the direction of using functions-

https://godbolt.org/z/MSH-a5

They will be as small as defines, you get the compiler to do some work which it does quite well, and also get the compiler to complain when you do things wrong instead of having to figure out what the preprocessor did with all the text substitutions.

The functions as static functions can go in a header, also. 

I think the less the preprocessor is involved, the better. Sometimes it seems there is no choice, but more often than you think it is not required and the compiler is more often a better tool.

If you want to test whether each bit of uint32_t data is set then why not

int main(void)
{
    uint32_t data = 0xaaaaaaaa;

    for (uint32_t mask = 1; mask != 0; mask <<= 1)
    {
        if (data & mask)
        {
            /* the bit was set */
        }
    }
    while (1);
}

You need to read up on bitwise AND and OR operators then. The basic boolean functions are AND,OR and NOT. Learn these and it will make sense.

I looked at #4 which is similar. I as going to contrast the naive method { (var & 1<<n) where n=0 to 31} against the special cases, but got sidetracked looking at the code for other processors. Several examples did recognise the intention of the special cases but produced quite lengthy code.

Anyway in brpl 1b:

1b just refers to a local label. In this case the closest label marked 1: looking backwards from the line.

Whereas in rjmp 2f:

The 2f means the closest label marked 2: forwards from the line.

{Edit: I forgot your 2nd question}

Indeed: I got that when modding the code for other processors. I guess it was included via <avr/io.h>

Adding #include <stdbool.h> does fix that.

>I am still very new to this but I think I got it. So these bit masks can I use them for the functions that curt showed above, will they reduce the processing time significantly?

https://godbolt.org/z/D8pt6X

You can try all 3 versions in 'main' by changing a define. Look at the assembly output. You can count instructions in the loop (some instructions take 1 clock, others 2), but the code size is normally a good indicator in many cases. They all end up similar enough. The downside to the mask code in this case, is it takes 128 bytes of flash, needs to use lpm to read flash, and does the masking on all 4 bytes which on an 8bit mcu is still not a great thing. That may work out better on a 32bit mcu, but you don't know until its tried.

>One thing more, can you make functions for uint16_t too

You just do the same thing as the 32bit version. You are taking in a var of some bit width (8,16,32,64) in the function argument, then finding/casting the byte of interest to an 8bit volatile by using the /8, and the bit of interest with the %8. In many cases of use, the compiler will have all the info needed so will be a few instructions (or 1 if using on something like &PORTB), and when it cannot know at compile time (like 'count' in the loop), it does a good job of coming up with code.

uint16_t a = 0x1234; //in memory as bytes -> 0x34, 0x12

bitset16( &a, 11 ); //we will say &a = 0x100 in this case

so 0x100+(11/8) = 0x101 (address of 0x12)

and 11%8 = 3

*(0x101) |= 1<<3; ->  0x12 |= 0x80, and we have just set bit 11 in a

static void bitset8(volatile uint8_t* reg, uint8_t bit){

  *reg |= 1<<bit;

}

static void bitset16(volatile uint16_t* reg, uint8_t bit){

  bitset8((volatile uint8_t*)reg+bit/8, bit%8);

}

static void bitset32(volatile uint32_t* reg, uint8_t bit){

  bitset8((volatile uint8_t*)reg+bit/8, bit%8);

}

static void bitset64(volatile uint64_t* reg, uint8_t bit){

  bitset8((volatile uint8_t*)reg+bit/8, bit%8);

}

>that it don't like bool ! 

The compiler explorer online compiler seems to use arduino backend, so my examples may be missing the required stdbool.h, and because it is already included somewhere else behind the scenes it works (I should include it in my examples because that would be the correct thing to do in any case).

Here is the same example compiled for a cortex-m4-

https://godbolt.org/z/MVPm2y

they all still look similar enough, so would not use the mask version there either as there is no clear advantage.

In most cases, you can create code that is simple to understand and the compiler will take over and translate to efficient code. There seems to be few places left where you have to 'persuade' the compiler to do something in a certain way to see a big benefit.

>The code in #7 take upto 61clk to check a bit

Seems to be in the range of 20's to 40's, but I could be wrong.

>The big question is why does OP want to use 32 bit in the first place (after all a AVR is 8bit).

There are 32 led's, so I assume there will be 32 bits of data to process in any case so really does not matter if it is 4 uint8_t's or one uint32_t. Probably the bigger problem it what is going on in the code after a decision is made on a bit (to presumably light an led, or turn it off).

It may end up one great big set of if's may be best if the led's are scattered about the ports where you have to deal with them one at a time. You would end up with a long line of cbi/sbi/sbrs/rjmp's, but in the end may be the quickest and possibly easiest. The advantage of dealing with a bit/pin at a time is you can easily change pins without having to change the bit patterns wherever they are coming from.

Since this is most likely just setting led patterns, time is probably not an issue in any case, and the slowest code you could come up with would probably work just fine. The mcu will probably still be in some loop doing nothing for 95% of the time.

I find this a particularly interesting "optimization."

The original bitfield/macro-based code lets you check any one bit of 32, very quickly.

Doing something with ALL the bits, one at a time, is a different optimization problem.

Finding "some" one bits is a 32bit string is yet a different problem.

And it tends to vary based on CPU architecture.  Any of the three problems I mentioned would be (best) done on an ARM CPU quite differently, because it's able to shift any number of bits in a single cycle.

I haven't tried building and examining assembler from all these different ways of doing it, but I I think it will largely depend on what the main usage is.

If the main usage is going to be setting/clearig individual bits one by one, then treating it as uin32_t like

void set_bit32(uint32_t *x, uint8_t bit)

{

    *x |= 1UL << bit;

}

is going to be relatively expensive like clawson said.

The compiler probably constructs the mask by looping around left shifting (or it might use add since you're just doubling the mask each time), so for a high bit num like 31 you do a lot of loops.

In this case, using curtvm method of breaking into 8bit chunks could well be faster (it relies on the endianess being correct but not a big deal since you probably aren't going to use this on other processors).

If, however, the main usage is going to be asssign a 32bit value pattern and then loop over all bits, then essentially you need to create a mask for all bit positions anyway.

So as long as you can convince the compiler to create the mask in a sensible manner (ie. just do one shift or add to the mask per iteration),  I think treating as a uint32_t would be pretty quick

eg.

uint32_t pattern = some 32 bit value;

uint8_t bit = 0;

for (uint32_t mask = 1; mask != 0; mask <<=1)

{

    if (pattern & mask)

    {

        some_function(bit);

    }

    bit++;

}

Yes, but that's the easy bit, and I can do that locally without having to go to some website and enable a bunch of javascript.

It's quite a neat site though to be fair.

The harder bit is taking the time to work out, in each case, what the assembler is doing, how many times will it loop around, how many cycles is that, how much difference does it make if  it inlines the function compared to if it doesn't inline the function, is it affected by optimisation in a trivial test program that wouldn't apply in the general case etc.

The direct way :

    for(uint8_t count=0; count < 32; count++){
        if( data & (1UL<<count)) 
          PORTB = count;
    }
 

This one would make a rather slow code for worst case (237clk), and rather big. It's because it insist on making complete 32bit calculations.  

00000063  LDI R24,0x00        Load immediate 
00000064  LDI R25,0x00        Load immediate 
        if( data & (1UL<<count)) 
00000065  LDS R20,0x0100        Load direct from data space 
00000067  LDS R21,0x0101        Load direct from data space 
00000069  LDS R22,0x0102        Load direct from data space 
0000006B  LDS R23,0x0103        Load direct from data space 
0000006D  MOV R0,R24        Copy register 
0000006E  RJMP PC+0x0005        Relative jump 
0000006F  LSR R23        Logical shift right 
00000070  ROR R22        Rotate right through carry 
00000071  ROR R21        Rotate right through carry 
00000072  ROR R20        Rotate right through carry 
00000073  DEC R0        Decrement 
00000074  BRPL PC-0x05        Branch if plus 
00000075  SBRC R20,0        Skip if bit in register cleared 
          PORTB = count;
00000076  OUT 0x05,R24        Out to I/O location 
00000077  ADIW R24,0x01        Add immediate to word 
    for(uint8_t count=0; count < 32; count++){
00000078  CPI R24,0x20        Compare with immediate 
00000079  CPC R25,R1        Compare with carry 
0000007A  BRNE PC-0x15        Branch if not equal 
 

for some reason it make count 16bit! 

add

For those that isn't used to the compiler output I guess I have to add that the compiler don't shift 1 up but rather shift the a copy of data down, and then check LSB. 

It give the correct result but is far from optimal on a AVR (I think it's because the new compiler frontends are made for ARM, I think the old AVR compiler would have made a better code)

From #26

Yes, if I read the assembler correctly, within each iteration of the for loop it is looping around count times, so effectively 'recreating' the mask on each iteration of the for loop.

You want it to 'remember' the mask from one iteration to the next, so then it just has to double the mask for each iteration. Yes, the mask is a 32 bit value so it has to do 4 shifts (or 4 adds) to double it, but that's not too bad.

For what it's worth, this code

int main(void)
{
    uint32_t data = get_32();
    uint8_t bit = 0;
    for (uint32_t mask = 1; mask != 0; mask <<= 1)
    {
        if (data & mask)
        {
            my_bit_func(bit);
        }
        bit++;
    }

    while (1);
}

produces

00000098 <main>:
  98:	0e 94 40 00 	call	0x80	; 0x80 <get_32>
  9c:	4b 01       	movw	r8, r22
  9e:	5c 01       	movw	r10, r24
  a0:	c1 2c       	mov	r12, r1
  a2:	d1 2c       	mov	r13, r1
  a4:	76 01       	movw	r14, r12
  a6:	c3 94       	inc	r12
  a8:	c0 e0       	ldi	r28, 0x00	; 0
  aa:	d7 01       	movw	r26, r14
  ac:	c6 01       	movw	r24, r12
  ae:	88 21       	and	r24, r8
  b0:	99 21       	and	r25, r9
  b2:	aa 21       	and	r26, r10
  b4:	bb 21       	and	r27, r11
  b6:	89 2b       	or	r24, r25
  b8:	8a 2b       	or	r24, r26
  ba:	8b 2b       	or	r24, r27
  bc:	19 f0       	breq	.+6      	; 0xc4 <main+0x2c>
  be:	8c 2f       	mov	r24, r28
  c0:	0e 94 4a 00 	call	0x94	; 0x94 <my_bit_func>
  c4:	cf 5f       	subi	r28, 0xFF	; 255
  c6:	cc 0c       	add	r12, r12
  c8:	dd 1c       	adc	r13, r13
  ca:	ee 1c       	adc	r14, r14
  cc:	ff 1c       	adc	r15, r15
  ce:	c0 32       	cpi	r28, 0x20	; 32
  d0:	61 f7       	brne	.-40     	; 0xaa <main+0x12>
  d2:	ff cf       	rjmp	.-2      	; 0xd2 <main+0x3a>

The loop starts at 0xaa, and there is at least no looping within the loop this time.

Well, the title of the thread is "How to check all bits in uint32_t" and the very first post shows looping over all 32 bits!

So who knows.

I just thought it's intersting to look at different ways of doing things.

For setting individual bits, if you can afford 8 byes of ram for array of 8bit masks, then using curtvm method but with 1u << bit%8  replaced with mask8[bit%8], that's about the best I can come up with.

static const uint8_t mask8[] = {1,2,4,8,16,32,64,128};

void setbit32(uint32_t *x, uint8_t b)
{
    /* assumes little endian */
    *((uint8_t *)x + b/8) |= mask8[b%8];
}

The compiler I have here gives

000000aa <setbit32>:
  aa:	26 2f       	mov	r18, r22
  ac:	26 95       	lsr	r18
  ae:	26 95       	lsr	r18
  b0:	26 95       	lsr	r18
  b2:	dc 01       	movw	r26, r24
  b4:	a2 0f       	add	r26, r18
  b6:	b1 1d       	adc	r27, r1
  b8:	67 70       	andi	r22, 0x07	; 7
  ba:	e6 2f       	mov	r30, r22
  bc:	f0 e0       	ldi	r31, 0x00	; 0
  be:	e0 50       	subi	r30, 0x00	; 0
  c0:	ff 4f       	sbci	r31, 0xFF	; 255
  c2:	8c 91       	ld	r24, X
  c4:	90 81       	ld	r25, Z
  c6:	98 2b       	or	r25, r24
  c8:	9c 93       	st	X, r25
  ca:	08 95       	ret

The example in #32 doesn't need any push/pop since it uses only call_used registers (I'm talking about  as part of a C program).

Or are you referring to the calling code?

The example from #28 has the 32bit value entirely in registers, r22-25, which is then copied to r8-r11.

You maybe can avoid the copy, but either way it is entirely in registers.

I'm not interested in saving a handful of clock cycles.

What I did find interesting was recognising that with uint32_t data you could have about an order of magnitude difference between

data |= 1uL << bit

vs writing it (yes still as C code) slightly differently.

It's easy to forget the limitations sometimes.

Compare to for example ARM instruction set where you get shifts for free.

>This loop is in a function with takes 1 uint32_t data which has 1's and 0's for 32 LEDs and further arguments for Start pwm value to end pwm value for each bit. A lot of data I don't know how to handle.

> but I might be wasting a lot of them somewhere else, giant switch , if else statements 

Instead of now heading off in another direction, why not decide what exactly you want to do first. See #21.

If you want pwm data for each bit, there is no need for bit testing as the pwm value has all the info and there is most likely no need for any big switch/if either. You would simply be sending out pwm data from some source-

uint8_t myData[16384]; //flash, external, wherever led data comes from

void sendLedHFrame(uint8_t* data){

    for(uint8_t i = 0; i < 16; i++){

        //do something with data[i] (pwm value)

        //maybe 8bit pwm value to 12 bit pwm off register, with on register at 0

        //like i2c send- 0 0 data[i]>>4 data[i]<<4

    }

}

void sendLedFrame(uint8_t* data){

    //I2C start, ic1 led0-15

    sendLedHFrame(data);

    //ic2 finish

    //I2C start, ic2 led16-31

    sendLedHFrame(data+16);

    //i2c finish

}

int main(){

    //setup

    for( int idx = 0;;){

        sendLedFrame(&myData[idx]);

        _delay_ms(50); //whatever frame rate wanted

        idx += 32;

        if( idx > sizeof(myData) ) idx = 0;

    }

}