Register access in IAR and Code Composer Studio (bitfields versus "standard" access)

Flavio Renga

Prodigy 110 points

I've just concluded the development of an application using the IAR IDE.

I included the io430g2553.h header file to write the code using a bitfield approach.

Hereafer a couple of examples of definition I used to access individual bits in the MSP registers:

#define SWITCH_IN P2IN_bit.P6 // P2.6
#define AUDIO_AMPLIFIER_ENABLE_OUT P2OUT_bit.P7 // P2.7

I'm now starting the migration to the Code Composer Studio IDE (I'm new with this IDE).

I only found the msp4302553.h header file: it seems it isn't possible to access the registers using a bit field approach.

Hereafter my questions:

Is there any procedure to use the already developed code "as it is"?
Should I modify ALL the code using the "standard" syntax?
There is any reason to avoid this the bitfield syntax? (I started years ago using bitfield approach because in my opinion the code is more readable)

Thanks in advance

over 11 years ago

0 Leo Hendrawan over 11 years ago

TI__Guru 56650 points

Hi Flavio,

i have been working with MSP430 since almost two years, and most of the time, i rarely see codes using bitfields, and also it is not really recommended in C:

http://c-faq.com/struct/bitfields.html

So unfortunatelly, i am afraid if you want to migrate to CCS, you have to modify your code by using the bitmasks definitions in the header file. The good thing is that it is also readable as with the bitfield.

0 Flavio Renga over 11 years ago in reply to Leo Hendrawan

Prodigy 110 points

Thank you for the fast reply, so I will start using bitmasks.
:-)

I heard that it's not recommended using macros to perform this task, e.g.:

#define setbit(A,B) ((A)|=(B))
#define clearbit(A,B) ((A)&=~(B))
#define testbit(A,B) ((A)&(B))
#define togglebit(A,B) ((A)^=(B))

Is it true?

So, what is the best approach between the code A and the code B?

Code A

setbit(P1OUT, BIT0)
setbit(P2OUT, (BIT0|BIT1))

Code B

P1OUT |= BIT0;
P2OUT |= (BIT0|BIT1);

Thanks

Flavio

0 Leo Hendrawan over 11 years ago in reply to Flavio Renga

TI__Guru 56650 points

Hi Flavio,

i think it should be ok to use the macros above, because you are using it correctly with the brackets. i can't think of any cases that using the macros would lead to an unexpected result (again because you are doing it with the brackets). However the example codes given for MSP430 usually use the second approach of "Code B". But in the end, i think this just a matter of programming taste :)

0 Flavio Renga over 11 years ago in reply to Leo Hendrawan

Prodigy 110 points

Thanks again Leo!

Flavio

0 Jens-Michael Gross over 11 years ago

Guru 227245 points

Use of bitfields is rather ineffective. They are usually use don hardware registers which are declared volatile. So the compiler has to generate one access per bit assignment.
Normal & and | accesss to a register can be grouped and are way more effective.

Also, in many cases it is unimportant whether e.g. P2.6 returns 1 or 0x40. It's either zero or not.

So in most cases, P2IN_Bit.P6 can be translated into (P2IN&BIT6). In some (few) cases, it shoul dbe better translated into ((P2IN&BIT6) >>6). Which is what the compiler generates for the bitfield anyway.
It has the additional advantage that this is an rvalue. You cannot do SWITCH_IN = 1 anymore, which would have been allowed with the bitfield.

For outputs, things are a bit more complex:

#define AUDIO_AMPLIFIER_ENABLE_OUT(x) P2OUT= (P2OUT&~BIT7)|(x?BIT7:0)

This too isn't an lvalue but more or less what the compiler generates for the bitfields. Well, if you do the change, you'll get an error for each usage in teh code, whcih makes it easy to fix :)

For a new project, I'd suggest using
#define AUDIO_AMPLIFIER_ENABLE (P2OUT|=BIT7)
#define AUDIO_AMPLIFIER_DISABLE (P2OUT &= ~BIT7)

Flavio Renga said:
Is there any procedure to use the already developed code "as it is"?

Yes, you can more or less auto-generate bitfield definitions for all used registers. For one single project/MSP type, this is likely more effort than it's worth.

Flavio Renga said:
Should I modify ALL the code using the "standard" syntax?

That's the better approach.

Flavio Renga said:
There is any reason to avoid this the bitfield syntax?

See above. Mainly because of its inefficiency. But in some cases, ther emight be side-effects on registers which cannot be avoided using the bitfields. Or are obfuscated by using bitfields ("I didn't touch this bit, why is it gone?" - because access to an other member of the bitfield caused a read/write operation that in turn cleared other bits)

Flavio Renga said:
(I started years ago using bitfield approach because in my opinion the code is more readable)

In most cases, especially for user-defined bitfields, this may be true. However, as explaind above, when bitfields are more than plain memory access, things can become even less readable. And cause bugs that are difficult to track down then.
Also, embedded programming is first order about efficiency. Beauty of code is far down on the priority list.

0 Flavio Renga over 11 years ago in reply to Leo Hendrawan

Prodigy 110 points

I put down this macro to assign a predefined value to a subset of bits in a register:

#define ASSIGN_BIT_MASK(REG,VAL,MASK) ((REG)=(VAL)|(REG)&(~(MASK)))

For instance:

ASSIGN_BIT_MASK(TA0CTL, MC_0, MC1|MC0);

Is there any smarter expression?

0 Leo Hendrawan over 11 years ago in reply to Flavio Renga

TI__Guru 56650 points

Hi Flavio,

Flavio Renga said:

I put down this macro to assign a predefined value to a subset of bits in a register:

#define ASSIGN_BIT_MASK(REG,VAL,MASK) ((REG)=(VAL)|(REG)&(~(MASK)))

i would do it in this way:

#define ASSIGN_BIT_MASK(REG,VAL,MASK) do {REG &= ~(MASK); REG |= VAL;} while(0);

it is more readable, and i think your implementation is not 100% correct (CMIIW), it should be maybe like this:

#define ASSIGN_BIT_MASK(REG,VAL,MASK) ((REG)=(VAL)| ((REG)&(~(MASK)))

the reason is that i think the compiler will read it left to right, so without the brackets, your macro above will works:

- VAL | REG

- result above & (~(MASK))

and i don't think this is what you want. so i would do it my way above, since after compilation it will produce the same code, but much more readable (not too many brackets).

0 Jens-Michael Gross over 11 years ago in reply to Leo Hendrawan

Guru 227245 points

lhend said:
i would do it in this way:
#define ASSIGN_BIT_MASK(REG,VAL,MASK) do {REG &= ~(MASK); REG |= VAL;} while(0);

Please without the ';' at the end.

lhend said:
without the brackets, your macro above will works

Yes, it would always clear out the masked values at the end rather than clearing the original content and or'ing the new value in.

However, I'd prefer this version (with the proper brackets) of your proposal because your marco does two read-modify-wite acceses to the register. It does not require any intermediate storage in a processor register for doing the math, but two read/write operations may cause unwanted side-effects and leave the hardware register in a potentially dangerous intermediate state for five clock cycles.

0 Flavio Renga over 11 years ago in reply to Jens-Michael Gross

Prodigy 110 points

Thanks Jens-Michael Gross and Leo for your suggestions.

Flavio

0 TonyKao over 11 years ago in reply to Jens-Michael Gross

Genius 3770 points

I much prefer using enums and inlined functions than macros for register access, like so:

#include "msp430g2452.h"
#include <stdint.h>

typedef enum {
  SWITCH_IN = BIT6,
  AUDIO_AMPLIFIER_ENABLE_OUT = BIT7
} PIN_Def;

static inline void set_pin(uint8_t volatile * const port, PIN_Def pin) {
  *port |= pin;
}
static inline void clear_pin(uint8_t volatile * const port, PIN_Def pin) {
  *port &= ~pin;
}
static inline uint8_t test_pin(uint8_t volatile * const port, PIN_Def pin) {
  return *port & pin;
}
static inline void toggle_pin(uint8_t volatile * const port, PIN_Def pin) {
  *port ^= pin;
}

int main( void )
{
  set_pin(&P2OUT, SWITCH_IN);
  clear_pin(&P2OUT, AUDIO_AMPLIFIER_ENABLE_OUT);
  if(test_pin(&P2OUT, SWITCH_IN))
    toggle_pin(&P2OUT, AUDIO_AMPLIFIER_ENABLE_OUT);
}

And this is the generated assembly (from IAR):

main:
        BIS.B   #0x40, &0x29
        BIC.B   #0x80, &0x29
        BIT.B   #0x40, &0x29
        JNC     ??main_0
        XOR.B   #0x80, &0x29

Not only does it do away with all the macro precedence counting, it enforces static type checking on the pins, all with zero code overhead.

Anyway just my two-cents ;)

Tony

0 Jens-Michael Gross over 11 years ago in reply to TonyKao

Guru 227245 points

TonyKao said:
I much prefer using enums and inlined functions than macros for register access, like so:

Yes, enums are a nice thing if you use them right (as types). I use them all the time for status return values and similar things.
However, I'm not sure about typechecking. Due to automatic conversion of numeric types, the typechecking is less effective.
You can pass any other numeric type as parameter, even a different enum type.

At least MSPGCC 3.2.3 allows it without even a warning. I just tried with two different enum types and also int or constant parameters. All are accepted. So forget about the typechecking.

And if it comes to multiple-bit values, the set and clear functions won't work too and a more complex 'change' function is needed.

However, an inlined function, if supported by the compiler as expected (which isn't guaranteed), will ensure that even more complex, multi-instruction operations won't break an if/else, just like the do/while in the macro.
So while it is not necessarily better, it is at least equally good. :)

0 Dung Dang over 11 years ago in reply to Jens-Michael Gross

TI__Genius 17802 points

Folks,

looks like everyone's all clear on the usage of bitfield vs. bitmask, as well as macros/inline definitions. I just want to point out that the ULP Advisor tool also offers to check and flag any bitfield usage and recommends switching to bitmask. Here's the wiki page for this rule, ULP15.1

http://processors.wiki.ti.com/index.php/Compiler/diagnostic_messages/MSP430/1546

This thread has quite a bit of valuable discussion on the topic, so please feel free to contribute to the wiki page if you can see any improvements we can make there.

And for those of you who are GCC users, ULP Advisor is currently provided with CCS & IAR, but soon it will come out with a stand-alone version that supports GCC projects.

Regards,

Dung

0 Jens-Michael Gross over 11 years ago in reply to Dung Dang

Guru 227245 points

Teh 'Why is this happening' info is incomplete.

The compiler could combine several bitfield accesss into one instruction - if it weren't on a hardware register that is declared volatile.
It's not the bitfield per se, it's the volatile target that makes bitfield operations ineffective, because the compiler is forced to access the target as many times and in exactly the order the source code does it. Imagine a multi-bit field in a hardware register would first be cleared and then be set. This would result in an intermediate state that is possibly hazardous. OTOH, on some registers it is absolutely necessary to set or clear certain bits simultaneously. In a volatile target, the compiler can do so only if both operations are in the same assignment. Impossible on bitfields.

On self-defined bitfields on memory structures, these problems do not apply - unless the structure is used in main as well as in an ISR and therefore must be declared volatile.

Well, I also find it more readable to have all settings on a hardware register in one line, but that's personal preference. :)

**Attention** This is a public forum

MSP low-power microcontrollers

MSP low-power microcontroller forum

Register access in IAR and Code Composer Studio (bitfields versus "standard" access)