Best practice for getting the value of a bit field in a byte

Log in or register to post comments
Go To Last Post
40 posts / 0 new
Author
Message
smileymicros
smileymicros's picture
Level: Raving Lunatic
Joined: Wed. Nov 17, 2004
Posts: 6320 View posts
Location: Great Smokey Mountains.
#1
Posted by smileymicros: Wed. Oct 5, 2011 - 11:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

And for another 'Best practice' query guaranteed to piss-off somebody...

Let's say you want to get the value of a group of bits from a byte that you've divided up into fields such as:

// DIP switch masks
#define POLARITYMASK 	0x01	// 00000001
#define SPEEDMASK 	0x0E	// 00001110
#define PATTERNMASK	0xF0	// 11110000

You can extract the pattern with:

pattern = ((dip_value & PATTERNMASK) >> 4 );

But suppose you want to write a generic function for your I-don't-want-to-think library that can do this sort of thing without you having to refer back to the mask to see where it is located so that you know how many shifts to use so you write:

uint8_t get_masked_field_8(uint8_t value, uint8_t mask)
{
	uint8_t shift = 0;

	for(int i = 0; i < 8; i++)
	{
		if( ( (mask >> i) & 0x01 ) ) 
                {
                    shift = i; 
                   break;
                }
	}
	return( (value&mask) >> shift );
}

Then you can get the pattern by just calling:

pattern = get_mask_field(dip_value,PATTERNMASK);

And be done with it.

Is this a "˜best practice' algorithm for accomplishing this task or is their a better way?

Smiley

Tags:

AVR Microcontrollers, megaAVR and tinyAVR
Top
sternst
sternst's picture
Level: Raving Lunatic
Joined: Mon. Jul 23, 2001
Posts: 3500 View posts
Location: Rheine, Germany
Posted by sternst: Wed. Oct 5, 2011 - 11:43 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

smileymicros wrote:
Is this a ‘best practice’ algorithm for accomplishing this task or is their a better way?
If "better" means "more efficient", then you have to avoid variable shifts on an AVR.

uint8_t get_masked_field_8 (uint8_t value, uint8_t mask) {

    value &= mask;

    while (!(mask & 0x01)) {
        value >>= 1;
        mask  >>= 1;
    }

    return value;
}

With this code mask must not be 0.
To be on the safe side you can put a safety test at the beginning of the function.

Stefan Ernst

Top
luvocean1
luvocean1's picture
Level: Posting Freak
Joined: Tue. Dec 26, 2006
Posts: 1688 View posts
Location: Sydney, Australia
Posted by luvocean1: Thu. Oct 6, 2011 - 01:42 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would have thought for such things you should rather have a macro and not a function, efficiencywise...

whats wrong with variable shifts? Does it does one cycle to do one bit shift?

Could you something like:

#define BIT_MASK_FIELD(byte, pos)     ((byte & (1 << pos)) > 0)

//probably better to do 
]#define BIT_MASK_FIELD(byte, pos)     ((byte & (1 << pos)) != 0)
Top
skeeve
skeeve's picture
Level: Raving Lunatic
Joined: Sun. Oct 29, 2006
Posts: 5223 View posts
Posted by skeeve: Thu. Oct 6, 2011 - 04:42 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I took better to mean more legible in use without loss of efficiency.
To make real sure the compiler treats the shift as a constant:

#define bittoshift8(mask) ( \
    ((mask) & 0x01) ? 0 : ( \
    ((mask) & 0x02) ? 1 : ( \
    ((mask) & 0x04) ? 2 : ( \
    ((mask) & 0x08) ? 3 : ( \
    ((mask) & 0x10) ? 4 : ( \
    ((mask) & 0x20) ? 5 : ( \
    ((mask) & 0x40) ? 6 : 7 )))))))

#define fetchmasked8(byte, mask) (((byte) & (mask))>>bittoshift8(mask))

In my work, I generally didn't bother shifting.

Iluvatar is the better part of Valar.

Top
Jepael
Jepael's picture
Level: Raving Lunatic
Joined: Mon. May 24, 2004
Posts: 6279 View posts
Location: Tampere, Finland
Posted by Jepael: Thu. Oct 6, 2011 - 07:04 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well, I don't see the much use of a general function because they might be too slow. On a ARM with tens of MHz and, big software and a lot of general function use over values read over some IIC/SPI bus it does not really matter, but on small AVR it is unnecessary overhead of passing parameters when they are usually known compile-time.

I am in the favor of making that a macro of somesort, so it would be optimized at compile-time to have constant shift and mask values for stuff. Perhaps a (inline) function to read the dip switches from IO pins directly and return the shifted value or put the switch content into correct flag variables etc, if it is going to be used for dip switches, as the switches are most likely to be read in one place only during software execution (at boot, or in mainloop, or in periodic timer interrupt, but still just in one place).

And in general, usually consecutive bits are needed, which is a property that can be exploited, for example "read N bits starting from position M from this variable", but not always, so non-consecutive bits have to be processed differently anyway.

But in general use I would make it a macro like this:

#define get_bits(value, mask, shift) ((value&mask)>>shift)

And use it in code like this:

...
uint8_t dips=get_bits(PINB, 0x1e, 1);
...
or
...
#define read_dips() get_bits(PINB, 0x1e, 1)
uint8_t dips=read_dips();
..
or
..
#define HAL_dips PINB, 0x1e, 1
#define HAL_read(a) get_bits(a) // this is needed to take in 1 argument and call with 3 arguments
...
uint8_t dips=HAL_read(HAL_dips);

But still if the get_bits is a function, I'd make the read_dips as a (inline) function:
#define DIPINPORT PINB
#define DIPMASK 0x1e
#define DIPSHIFT 1
inline uint8_t read_dips(void)
{
  return ((DIPINPORT&DIPMASK)>>DIPSHIFT);
}

but that will easily end up in dependency hell so why not just write:

inline uint8_t read_dips(void)
{
  return ((PINB&0x1e)>>1);
}

Of course things are different when things are not known compile-time so then you do need to mask and shift with a variable.

Top
westfw
westfw's picture
Level: Raving Lunatic
Joined: Wed. Jun 19, 2002
Posts: 4460 View posts
Location: SF Bay area
Posted by westfw: Thu. Oct 6, 2011 - 08:51 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Let’s say you want to get the value of a group of bits from a byte

Do you really need to? Without a barrel shifter, and with adequate 8bit immediate instructions, you're a lot better off leaving the bits wherever they are shifting any relevant constants instead...

#define PATTERNMASK 0xC0
#define PATTERN_0 0x00
#define PATTERN_1 0x40
#define PATTERN_2 0x80
#define PATTERN_3 0xC0

if ((mybyte & PATTERNMASK) == PATTERN_2) ...
Top
Jepael
Jepael's picture
Level: Raving Lunatic
Joined: Mon. May 24, 2004
Posts: 6279 View posts
Location: Tampere, Finland
Posted by Jepael: Thu. Oct 6, 2011 - 09:42 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

westfw wrote:
Quote:
Let’s say you want to get the value of a group of bits from a byte

Do you really need to? Without a barrel shifter, and with adequate 8bit immediate instructions, you're a lot better off leaving the bits wherever they are shifting any relevant constants instead...

#define PATTERNMASK 0xC0
#define PATTERN_0 0x00
#define PATTERN_1 0x40
#define PATTERN_2 0x80
#define PATTERN_3 0xC0

if ((mybyte & PATTERNMASK) == PATTERN_2) ...

For relatively small amount of patterns, I do it the same way. Best example I can think of is the AVR TWI status, where five or so most significant bits define the status code, but not all 32 values are needed.

But this does not apply to case where the result is best to be used when unused bits are shifted away, for example indexing an array of function pointers.

And this does not apply at all for example when displaying a 18-bit ADC result residing in middle of 24 status bits.

Top
smileymicros
smileymicros's picture
Level: Raving Lunatic
Joined: Wed. Nov 17, 2004
Posts: 6320 View posts
Location: Great Smokey Mountains.
Posted by smileymicros: Thu. Oct 6, 2011 - 02:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks guys. I guess I need to redo this and test it to see the assembly generated before I make my final judgement.

And I too recall something about not using variables in shifts but my Google-Fu wasn't up to finding out why this is a problem. Okay, more coding and looking at the results.

Smiley

Top
Silver Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 99691 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Thu. Oct 6, 2011 - 02:28 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

test it to see the assembly generated before I make my final judgement.

I did - no question, use Michael's solution - I think it was only 4 opcodes!

	PORTB = fetchmasked8(PINC, PATTERNMASK);
  a6:	83 b3       	in	r24, 0x13	; 19
  a8:	82 95       	swap	r24
  aa:	8f 70       	andi	r24, 0x0F	; 15
  ac:	88 bb       	out	0x18, r24	; 24

Top
Koshchi
Koshchi's picture
Level: 10k+ Postman
Joined: Wed. Nov 17, 2004
Posts: 15674 View posts
Location: Vancouver, BC
Posted by Koshchi: Thu. Oct 6, 2011 - 03:01 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't see the problem with just using plain old C:

typedef struct
{
    uint8_t Polarity : 1;
    uint8_t Speed : 3;
    uint8_t Pattern : 4;
} DipValue;

DipValue value;

uint8_t pattern value.Pattern;

Regards,
Steve A.

The Board helps those that help themselves.

Top
Jepael
Jepael's picture
Level: Raving Lunatic
Joined: Mon. May 24, 2004
Posts: 6279 View posts
Location: Tampere, Finland
Posted by Jepael: Thu. Oct 6, 2011 - 04:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

C bitfields are useful, when their limitations are taken into account.

Order of the bits is compiler/CPU specific so if you use uint8_t type for the bits they might get swapped from hi/lo to lo/hi order.

Even bigger issue might be if uint16_t or uint32_t are used, what happens between big-endian and little-endian machines.

In fact here are even more good points I have not yet found out the hard way.

Top
Koshchi
Koshchi's picture
Level: 10k+ Postman
Joined: Wed. Nov 17, 2004
Posts: 15674 View posts
Location: Vancouver, BC
Posted by Koshchi: Thu. Oct 6, 2011 - 04:32 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Order of the bits is compiler/CPU specific so if you use uint8_t type for the bits they might get swapped from hi/lo to lo/hi order.

Even bigger issue might be if uint16_t or uint32_t are used, what happens between big-endian and little-endian machines.

How often do you change compilers? And which compilers for the AVR use different orderings? And if you change CPUs, you need to change your code regardless of what compiler you use. Quite frankly, on the AVR these points are moot.

Regards,
Steve A.

The Board helps those that help themselves.

Top
skeeve
skeeve's picture
Level: Raving Lunatic
Joined: Sun. Oct 29, 2006
Posts: 5223 View posts
Posted by skeeve: Thu. Oct 6, 2011 - 04:32 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

smileymicros wrote:
And I too recall something about not using variables in shifts but my Google-Fu wasn't up to finding out why this is a problem. Okay, more coding and looking at the results.
A variable shift requires a loop.
Loop overhead will at least quadruple the number of cycles.

Iluvatar is the better part of Valar.

Top
skeeve
skeeve's picture
Level: Raving Lunatic
Joined: Sun. Oct 29, 2006
Posts: 5223 View posts
Posted by skeeve: Thu. Oct 6, 2011 - 04:40 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The bit-field approach is probably ok for one's internal structures.
The precise locations of the fields are not important.
For something that connects to the outside world, e.g. PINB,
one must know how the compiler lays out bit-fields.
Thus, in this case, it fails on legibility grounds.

Iluvatar is the better part of Valar.

Top
hugoboss
hugoboss's picture
Level: Resident
Joined: Fri. Feb 19, 2010
Posts: 728 View posts
Location: Montreal, QC, CA
Posted by hugoboss: Thu. Oct 6, 2011 - 04:41 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
Dim vBitfield As Byte
    Bit0 Alias vBitfield.0
    Bit1 Alias vBitfield.1
    Bit2 Alias vBitfield.2
    Bit3 Alias vBitfield.3
    Bit4 Alias vBitfield.4
    Bit5 Alias vBitfield.5
    Bit6 Alias vBitfield.6
    Bit7 Alias vBitfield.7

...
Call DoSomething(Bit0)

For i = 0 to 7
    Call DoSomething(vBitfield.i)
Next i

So much easier to read...

Top
Silver Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 99691 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Thu. Oct 6, 2011 - 04:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just to note that assuming Joe's only writing this for avr-gcc then there's no doubt that it allocates the bitfields from bit 0 up to bit 7. Thus Steve's definition above is correct, hence:

#include 

typedef struct
{
    uint8_t Polarity : 1;
    uint8_t Speed : 3;
    uint8_t Pattern : 4;
} DipValue;

int main(void) {
  PORTB = (*(DipValue *)&PINC).Pattern;
}

which generates:

  PORTB = (*(DipValue *)&PINC).Pattern;
  6c:	83 b3       	in	r24, 0x13	; 19
  6e:	82 95       	swap	r24
  70:	8f 70       	andi	r24, 0x0F	; 15
  72:	88 bb       	out	0x18, r24	; 24

This is the exact same opcode sequence as Michael's macro generated.

(if this is for a magazine article it could be fun explaining that syntax to a beginner ;-))

Quote:
So much easier to read...

Now add the bit that actually conglomerates the 4 bits of "pattern" read from the DIP switch port!

Top
Koshchi
Koshchi's picture
Level: 10k+ Postman
Joined: Wed. Nov 17, 2004
Posts: 15674 View posts
Location: Vancouver, BC
Posted by Koshchi: Thu. Oct 6, 2011 - 05:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
For something that connects to the outside world, e.g. PINB, one must know how the compiler lays out bit-fields.
One must know how the connections to the outside world map to the bits in the C variable. This is true no matter what method is used to access those bits.
Quote:
Thus, in this case, it fails on legibility grounds.
I fail to see how your point relates to legibility at all. The value is accessed as value.Pattern regardless of how the bit fields are ordered.
Quote:
Just to note that assuming Joe's only writing this for avr-gcc then there's no doubt that it allocates the bitfields from bit 0 up to bit 7.
I still would like someone to point out any C compiler that does it differently.

Regards,
Steve A.

The Board helps those that help themselves.

Top
smileymicros
smileymicros's picture
Level: Raving Lunatic
Joined: Wed. Nov 17, 2004
Posts: 6320 View posts
Location: Great Smokey Mountains.
Posted by smileymicros: Thu. Oct 6, 2011 - 06:10 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
(if this is for a magazine article it could be fun explaining that syntax to a beginner ;-))
I think I'll go with Michael's original version since there is a finite chance in Hell that I can explain it in a novice friendly article. I'll reserve your version for my forthcoming book: "C for Gurus with Gianormous Cojones".

Thanks,
Smiley

Top
hobbss
hobbss's picture
Level: Resident
Joined: Wed. Oct 6, 2010
Posts: 989 View posts
Location: Detroit
Posted by hobbss: Thu. Oct 6, 2011 - 06:54 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A title like that would require at least two more levels of misdir... err... indirection.

Science is not consensus. Science is numbers.

Top
Jepael
Jepael's picture
Level: Raving Lunatic
Joined: Mon. May 24, 2004
Posts: 6279 View posts
Location: Tampere, Finland
Posted by Jepael: Thu. Oct 6, 2011 - 07:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Koshchi wrote:
I still would like someone to point out any C compiler that does it differently.

Sorry I did not mean different compilers for AVR, I meant different compilers in general. If I make a header that defines some data structure with bitfields, and I transfer that data structure as-is through serial port to another device (PC, ARM, 8051, HC11, PIC, whatever) that has used the same header, it either works or it does not, and using variables larger than 8 bits increase the possibility it fails. Inside AVR and for user variables the order is of course not important.

I have shot myself in the foot by porting 8051-compatible code to ARM, and of course it did not work - fortunately not due to use of bitfields but the word endianness in memory.

This affects structures in general, as for example GCC compiler generating Windows executables may align structures to machine word boundaries when it makes them faster to access, so same structure on AVR does not necessarily work on PC, unless you add the keyword "packed" to the PC side definition of the structure.

Top
skeeve
skeeve's picture
Level: Raving Lunatic
Joined: Sun. Oct 29, 2006
Posts: 5223 View posts
Posted by skeeve: Thu. Oct 6, 2011 - 08:07 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Koshchi wrote:
Quote:
For something that connects to the outside world, e.g. PINB, one must know how the compiler lays out bit-fields.
One must know how the connections to the outside world map to the bits in the C variable. This is true no matter what method is used to access those bits.
Quote:
Thus, in this case, it fails on legibility grounds.
I fail to see how your point relates to legibility at all. The value is accessed as value.Pattern regardless of how the bit fields are ordered.
To use bit-fields, the coder has to know what bit-fields to use and the maintainer has to be able to tell that the coder did it correctly.
Quote:
Quote:
Just to note that assuming Joe's only writing this for avr-gcc then there's no doubt that it allocates the bitfields from bit 0 up to bit 7.
I still would like someone to point out any C compiler that does it differently.
Possibly there aren't any, but that doesn't matter.
What matters is that there are at least two sensible ways for the structure to be laid out:
       Pattern Speed Polarity
       ------- ----- -
       7 6 5 4 3 2 1 0
       - ----- -------
Polarity Speed Pattern

They require thinking that is not required with explicit masks.

Iluvatar is the better part of Valar.

Top
Silver Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 99691 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Thu. Oct 6, 2011 - 08:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

To use bit-fields, the coder has to know what bit-fields to use and the maintainer has to be able to tell that the coder did it correctly.

This whole thread started with:

// DIP switch masks
#define POLARITYMASK    0x01   // 00000001
#define SPEEDMASK    0x0E   // 00001110
#define PATTERNMASK   0xF0   // 11110000

so surely the layout of the bits was set in stone by that however you choose to then split these out of the 8 you read from the port?

Armed with this and the knowledge that the C compiler stores bitfields from 0 onwards surely there's no "maintenance nightmare here". If there's any doubt put a comment in for the maintainers benefit to explain the bit layout in the struct.

Top
smileymicros
smileymicros's picture
Level: Raving Lunatic
Joined: Wed. Nov 17, 2004
Posts: 6320 View posts
Location: Great Smokey Mountains.
Posted by smileymicros: Thu. Oct 6, 2011 - 09:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Set in stone would be a one off task, but I wanted a more general solution for both educational reasons and practical ones such as the 'oh-shit-i-messed-up-the-pcb-and-must-revise-the-masks-or-get-canned'.

BTW - I tested these on Pelles C on a PC and they work just fine so I assume that compilers won't often be and issue.

Top
skeeve
skeeve's picture
Level: Raving Lunatic
Joined: Sun. Oct 29, 2006
Posts: 5223 View posts
Posted by skeeve: Thu. Oct 6, 2011 - 09:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
This whole thread started with:

// DIP switch masks
#define POLARITYMASK    0x01   // 00000001
#define SPEEDMASK    0x0E   // 00001110
#define PATTERNMASK   0xF0   // 11110000

so surely the layout of the bits was set in stone by that however you choose to then split these out of the 8 you read from the port?

Armed with this and the knowledge that the C compiler stores bitfields from 0 onwards surely there's no "maintenance nightmare here". If there's any doubt put a comment in for the maintainers benefit to explain the bit layout in the struct.

I agree that he bit-field method is not a maintenance nightmare.
Remember the measure of success:
smileymicros wrote:
But suppose you want to write a generic function for your I-don't-want-to-think library that can do this sort of thing without you having to refer back to the mask to see where it is located so that you know how many shifts to use so you write:
Emphasis added.
The bit-field method requires more thought.
The syntax is a bit more complicated and the struct requires a name.

Iluvatar is the better part of Valar.

Top
hugoboss
hugoboss's picture
Level: Resident
Joined: Fri. Feb 19, 2010
Posts: 728 View posts
Location: Montreal, QC, CA
Posted by hugoboss: Thu. Oct 6, 2011 - 11:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
' This table references the character values for the seven segment cathodes on PORTA and PORTD.
Dim Display_number(10) As Byte
    Display_number(1) = &B11111001 ' Number 1  - B,C
    Display_number(2) = &B10100100 ' Number 2  - A,B,D,E,G
    Display_number(3) = &B10110000 ' Number 3  - A,B,C,D,G
    Display_number(4) = &B10011001 ' Number 4  - B,C,F,G
    Display_number(5) = &B10010010 ' Number 5  - A,C,D,F,G
    Display_number(6) = &B10000010 ' Number 6  - A,C,D,E,F
    Display_number(7) = &B11111000 ' Number 7  - A,B,C
    Display_number(8) = &B10000000 ' Number 8  - A,B,C,D,E,F,G
    Display_number(9) = &B10010000 ' Number 9  - A,B,C,D,F,G
    Display_number(10) = &B11000000' Number 0  - A,B,C,D,E,F

' This table references the phase values for the common anodes on PORTF.
Dim Display_phasebyte(5) As Byte 
    Display_phasebyte(1) = &B11111110 ' Phase 1 - Digit 1
    Display_phasebyte(2) = &B11111101 ' Phase 2 - Digit 2
    Display_phasebyte(3) = &B11111011 ' Phase 3 - Digit 3
    Display_phasebyte(4) = &B11110111 ' Phase 4 - Digit 4
    Display_phasebyte(5) = &B11101111 ' Phase 5 - Semicolon & Team Marker

I find bitfields much more clear and concise than shifts and rotates... In the end you are really just trading memory for running cycles, so choose as needed.

Top
stevech
stevech's picture
Level: Raving Lunatic
Joined: Tue. Dec 18, 2001
Posts: 6148 View posts
Posted by stevech: Fri. Oct 7, 2011 - 04:12 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The compiler I use supports the ANSI C99 standard and in that are means to create bitfields in a selected data type, such as a byte rather than an int. I think GCC has long resisted supporting C99 but most commercial compilers do. Another good part of C99 is designated initializers , which really simplify initializing structures.

To me, code using bitfields is less bug-prone and self-documenting, e.g.,

varx.flagbitA = 1;
is better than
varx |= (1<<K); // where K is the bit number 0..n
or worse
varx &= ~(1<<K);

and a declaration - which leads to ONE byte of storage..

typedef struct {
  uint8_t                                :1; // bit 0
  uint8_t                                :1; // bit 1
  uint8_t                                :1; // bit 2
  uint8_t                                :1; // bit 3
  uint8_t conditionx                     :1; // bit 4
  uint8_t conditiony                     :1; // bit 5
  uint8_t flagAlpha                      :1; // bit 6
  uint8_t flagBeta                       :1; // bit 7
} byteOfFlagsCase1_t;

...
byteOfFlagsCase1_t varx;
...
varx.conditiony = 1;  // simple, eh?

I've seen optimizing in a compiler get very efficient bit bit set/clear when bitfields are used.

Top
Bronze MarkJohanEkdahl
JohanEkdahl's picture
Level: 10k+ Postman
Joined: Wed. Mar 27, 2002
Posts: 26988 View posts
Location: Lund, Sweden
Posted by JohanEkdahl: Fri. Oct 7, 2011 - 05:02 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
I think GCC has long resisted supporting C99 but most commercial compilers do.

I thought GCC supported C99...

EDIT: But you live to learn: http://gcc.gnu.org/gcc-4.6/c99st...

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here. 

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

Top
meslomp
meslomp's picture
Level: Raving Lunatic
Joined: Wed. May 2, 2007
Posts: 6044 View posts
Location: Nieuwegein, Netherlands
Posted by meslomp: Fri. Oct 7, 2011 - 05:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Set in stone would be a one off task, but I wanted a more general solution for both educational reasons and practical ones such as the 'oh-shit-i-messed-up-the-pcb-and-must-revise-the-masks-or-get-canned'.

you always have to do that. When you change something on the board you always have to check your firmware to ensure that it did not impact that.
I always have a 'board.h' file that holds all the defines regarding my PCB. so when I need to change the PCB i just have to check and possibly change that single file to get my firmware into sink again with the new PCB status.
It worked up till now for me...

and as a reasonable novice in programming: on one side it is better understandable to use bit masks and then using them, but I lately also came accross the usage of a struct to have a set of bits defined in a byte and that also was an eye opener. But for a novice it can be hard to debug when on one side you use the entire structure to put data in and on the other side filter-out bits. The chance of making a mistake there or forgetting to change a 'link' when you changed the bit pattern will be significant.

But you could for the article just start with using the masks and then evolve that to using the bit fields in a byte structure.

Top
westfw
westfw's picture
Level: Raving Lunatic
Joined: Wed. Jun 19, 2002
Posts: 4460 View posts
Location: SF Bay area
Posted by westfw: Fri. Oct 7, 2011 - 06:21 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
I still would like someone to point out any C compiler that does it differently.

gcc for 68k is consistently big-endian, and assigns bitfields starting from the high bits.

But this is a "standard" portability problem, and I wouldn't worry a lot about using structure-based bitfields in an AVR program.

Did you know that you can't use structure-based bitfields to describe the Fragment Offset field of an IP packet on a little-endian byte-addressed machine?

Top
Silver Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 99691 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Fri. Oct 7, 2011 - 08:50 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I think GCC has long resisted supporting C99 but most commercial compilers do.

What are -std=c99 (and the even better -std=gnu99) then?!?

Also explain to me exactly how much support Microsoft Visual C++ has for C99? (I just naturally tried to use designated initializers then after a hunt found that M$ have chosen not to support C99!). See this thread:

https://www.avrfreaks.net/index.p...

(actually you posted in that thread?)

Top
skeeve
skeeve's picture
Level: Raving Lunatic
Joined: Sun. Oct 29, 2006
Posts: 5223 View posts
Posted by skeeve: Fri. Oct 7, 2011 - 05:27 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

stevech wrote:
To me, code using bitfields is less bug-prone and self-documenting, e.g.,

varx.flagbitA = 1;
is better than
varx |= (1<<K); // where K is the bit number 0..n
or worse
varx &= ~(1<<K);

You're solving the wrong problem:
Smiley's starting point was a set of masks and a desire to fetch.
Assignments are not wanted.
#include "fetchmasked8.h"
// DIP switch masks
#define POLARITYMASK    0x01   // 00000001
#define SPEEDMASK    0x0E   // 00001110
#define PATTERNMASK   0xF0   // 11110000

requires less thought than

Quote:

typedef struct {
  uint8_t                                :1; // bit 0
  uint8_t                                :1; // bit 1
  uint8_t                                :1; // bit 2
  uint8_t                                :1; // bit 3
  uint8_t conditionx                     :1; // bit 4
  uint8_t conditiony                     :1; // bit 5
  uint8_t flagAlpha                      :1; // bit 6
  uint8_t flagBeta                       :1; // bit 7
} byteOfFlagsCase1_t;

...
byteOfFlagsCase1_t varx;
...
varx.conditiony = 1;  // simple, eh?

That said, when one starts with sizes, wants to perform assignments and does not need to connect directly to the outside world, bit-fields are the way to go.

Iluvatar is the better part of Valar.

Top
Koshchi
Koshchi's picture
Level: 10k+ Postman
Joined: Wed. Nov 17, 2004
Posts: 15674 View posts
Location: Vancouver, BC
Posted by Koshchi: Sat. Oct 8, 2011 - 04:44 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
You're solving the wrong problem:
Smiley's starting point was a set of masks and a desire to fetch.
Assignments are not wanted.
The bit fields solves BOTH problems. Is that a reason to not use it?
Quote:
requires less thought than
But that struct does not match the defines. Using the struct that matches the defines there is no more thought whatsoever. The only additional information needed the order that the compiler fills the bits.

And your ignoring the fact that you need this monstrosity:

#define bittoshift8(mask) ( \ 
    ((mask) & 0x01) ? 0 : ( \ 
    ((mask) & 0x02) ? 1 : ( \ 
    ((mask) & 0x04) ? 2 : ( \ 
    ((mask) & 0x08) ? 3 : ( \ 
    ((mask) & 0x10) ? 4 : ( \ 
    ((mask) & 0x20) ? 5 : ( \ 
    ((mask) & 0x40) ? 6 : 7 ))))))) 

#define fetchmasked8(byte, mask) (((byte) & (mask))>>bittoshift8(mask))

in order to use it as easily as the bit field.

Regards,
Steve A.

The Board helps those that help themselves.

Top
smileymicros
smileymicros's picture
Level: Raving Lunatic
Joined: Wed. Nov 17, 2004
Posts: 6320 View posts
Location: Great Smokey Mountains.
Posted by smileymicros: Sat. Oct 8, 2011 - 09:16 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But do the bit-fields optimize down as well as Michael's algorithm? Bit fields are conceptually clearer but I thought there were other problems, at least with using them with AVR-gcc. Okay, I guess I'll do it both ways and see...

Smiley

Top
Silver Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 99691 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Sun. Oct 9, 2011 - 11:28 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

But do the bit-fields optimize down as well as Michael's algorithm?

yes, I already showed that Steve's bitfield solution actually generated exactly the same 4 opcodes!

I'm not aware of any issue with using bitfields in GCC?!?

Top
smileymicros
smileymicros's picture
Level: Raving Lunatic
Joined: Wed. Nov 17, 2004
Posts: 6320 View posts
Location: Great Smokey Mountains.
Posted by smileymicros: Sun. Oct 9, 2011 - 02:56 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Okay, yeah - thanks as usual. I guess I stumbled after the example with all the indirections imagining trying to explain that to folks.

But somewhere sometime I got it in my head that using bitfields with avr-gcc isn't a good practice because of problems with promotions (or something I can't remember at the moment). If there are no problems with bitfields then I should start using them since they are by far the clearest way to illustrate these points. I'll do some more digging but if anybody knows about particular problems with bitfields I'd like to hear about them.

Smiley

Top
Silver Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 99691 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Sun. Oct 9, 2011 - 03:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

then I should start using them since they are by far the clearest way to illustrate these points.

From another recent thread:

#include 

typedef struct {
        union {
          uint8_t ADPS0:1,
            ADPS1:1,
            ADPS2:1,
            ADIE:1,
            ADIF:1,
            ADATE:1,
            ADSC:1,
            ADEN:1;
          uint8_t byte;
        };
} ADC_ctrl_t;


// mega48/88/168
#define Adcsra (*(volatile ADC_ctrl_t *)0x7A)

int main(void){
        Adcsra.ADEN = 1;
        Adcsra.byte = 0x5A;
}

One could expand that out to all the SFRs/bits and for the more traditional kind of access (like when you want to set several bits at once) you just use the ".byte" field.

In the example I use Adcsra rather than ADCSRA so as not to name clash with if you choose to use it for other things. If you did include I gues the define could be:

#define Adcsra (*(volatile ADC_ctrl_t *)ADCSRA)

Top
Brutte
Brutte's picture
Level: Raving Lunatic
Joined: Thu. Oct 5, 2006
Posts: 3231 View posts
Location: Poland
Posted by Brutte: Sun. Oct 9, 2011 - 04:39 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

smileymicros wrote:
Let’s say you want to get the value of a group of bits from a byte that you’ve divided up into fields

on AVRs I guess.
As you have already remarked you didn't provide the "best" meaning, the performance index to minimize - is it supposed to be optimized for speed, flash, sram usage? Or for the least number of brackets when written in Ada95?
smileymicros wrote:
But suppose you want to write a generic function

Are some of the arguments (shift, mask, struct size) supposed to be known at compile time? What about multibyte struct with some 11-bit bitfield? How "generic" is your function? Do you want it to have the variable input structure size, so that it managed to extract an x-bit bitfield variable form y-byte structure at z-bit offset(x,y,z is variable)?

A called function will be the smallest in terms of flash (if called n-times) in such case.
And you will not find a faster solution than LUT (if you can afford this amount of SRAM/FLASH).

No RSTDISBL, no fun!

Top
skeeve
skeeve's picture
Level: Raving Lunatic
Joined: Sun. Oct 29, 2006
Posts: 5223 View posts
Posted by skeeve: Sun. Oct 9, 2011 - 08:27 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Koshchi wrote:
Quote:
You're solving the wrong problem:
Smiley's starting point was a set of masks and a desire to fetch.
Assignments are not wanted.
The bit fields solves BOTH problems. Is that a reason to not use it?
Quote:
requires less thought than
But that struct does not match the defines. Using the struct that matches the defines there is no more thought whatsoever. The only additional information needed the order that the compiler fills the bits.
I'm not sure what the point is here.
Mine is that no mechanism was provided to go from a set of masks to bit-field sizes and bit-field order.
Someone starting from masks has to think to produce them.
The starting point matters.
Quote:
And your ignoring the fact that you need this monstrosity:

#define bittoshift8(mask) ( \ 
    ((mask) & 0x01) ? 0 : ( \ 
    ((mask) & 0x02) ? 1 : ( \ 
    ((mask) & 0x04) ? 2 : ( \ 
    ((mask) & 0x08) ? 3 : ( \ 
    ((mask) & 0x10) ? 4 : ( \ 
    ((mask) & 0x20) ? 5 : ( \ 
    ((mask) & 0x40) ? 6 : 7 ))))))) 

#define fetchmasked8(byte, mask) (((byte) & (mask))>>bittoshift8(mask))

in order to use it as easily as the bit field.

The monstrosity only needs to be written once.
From then on it can be #included.
The bit-field method requires defining a struct for every set of masks.

Iluvatar is the better part of Valar.

Top
skeeve
skeeve's picture
Level: Raving Lunatic
Joined: Sun. Oct 29, 2006
Posts: 5223 View posts
Posted by skeeve: Sun. Oct 9, 2011 - 08:50 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Brutte wrote:
smileymicros wrote:
Let’s say you want to get the value of a group of bits from a byte that you’ve divided up into fields

on AVRs I guess.
As you have already remarked you didn't provide the "best" meaning, the performance index to minimize - is it supposed to be optimized for speed, flash, sram usage? Or for the least number of brackets when written in Ada95?

It seems to me that he did:
smileymicros wrote:
But suppose you want to write a generic function for your I-don't-want-to-think library that can do this sort of thing without you having to refer back to the mask to see where it is located so that you know how many shifts to use so you write:
Emphasis added again.
Quote:
smileymicros wrote:
But suppose you want to write a generic function
Are some of the arguments (shift, mask, struct size) supposed to be known at compile time? What about multibyte struct with some 11-bit bitfield?
Brutte wrote:
smileymicros wrote:
Let’s say you want to get the value of a group of bits from a byte that you’ve divided up into fields
Emphasis added.

Iluvatar is the better part of Valar.

Top
smileymicros
smileymicros's picture
Level: Raving Lunatic
Joined: Wed. Nov 17, 2004
Posts: 6320 View posts
Location: Great Smokey Mountains.
Posted by smileymicros: Fri. Dec 23, 2011 - 09:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

What I finally decided:

At first I used Michael's method, but later I switched to:

union{
	struct {
		uint8_t polarity : 1;	// 00000001
		uint8_t speed : 2;		// 00000110
		uint8_t pattern: 5;		// 11111000
	};
	uint8_t dipvalue;
}dip;

Since it gives me the option of directly reading the DIP into dipvalue and automatically having the pattern loaded and accessible as follows:

dip.dipvalue = read_dip();
		
set_speed(dip.speed);
set_polarity(dip.polarity);
set_pattern(dip.pattern);

I think this is clearer to the less experienced programmer and it seems to give exactly the same code size as the other technique. It also provides a good illustration of using structs, unions, and bit-fields.

Thanks for all the input.

BTW - I am aware of the portability problem -
From K&R 2nd Edition p150:

Quote:

Almost everything about fields is implementation-dependent. Whether a field may overlap a word boundary is implementation-defined. Fields need not be named; unnamed fields (a colon and a width only) are used for padding. The special width 0 may be used to force alignment at the next word boundary.

Fields are assigned left to right on some machines and right to left on others. This means that although fields are useful for maintaining internally-defined data structures, the question of which end comes first has to be carefully considered when picking apart externally-defined data: programs that depend on such things are not portable. Fields may be declared only as ints; for portability, specify signed or unsigned explicitly. Thea re not arrays, and they do not have address, so the & operator cannot be applied to them.


But I decided that I am unlikely to be using this for anything other than the AVR with gcc so it doesn't really matter for my application.

Top