Peter,

The change which caused code to compile differently involved the use of the (currently unsupported) iostructures.h file. Specifically the file contains the following definition to define a union of a bit field structure and an unsigned char:

   #ifdef MSP430_BITFIELDS_UNSIGNED
   #define __MSP430_UNS__
   #else
   #define __MSP430_UNS__ unsigned
   #endif

   #ifndef __MSP430_PORT_INIT_
   #define __VOLATILE volatile
   #else
   #define __VOLATILE
   #endif

   typedef union port {
      __VOLATILE __MSP430_UNS__ char reg_p;
      __VOLATILE struct {
        __MSP430_UNS__ char __p0:1,
          __p1:1,
          __p2:1,
          __p3:1,
          __p4:1,
          __p5:1,
          __p6:1,
          __p7:1;
      } __pin;
   } __attribute__ ((packed)) ioregister_t;

   #undef __MSP430_UNS__

When I compile this in to my code the ioregister_t type has a size of 2 bytes rather than 1 as if the incorporation of a bit field automatically promoted the type to int. Note that the "packed" attribute has been applied. This type is used subsequently in the same header to construct register sets for example:

   #if defined(__msp430_have_port1) || defined(__msp430_have_port2)
   struct port_full_t {
      ioregister_t    in;    /* Input */
      ioregister_t    out;    /* Output */
      ioregister_t    dir;    /* Direction */
      ioregister_t    ifg;    /* Interrupt Flag */
      ioregister_t    ies;    /* Interrupt Edge Select */
      ioregister_t    ie;    /* Interrupt Enable */
      ioregister_t    sel;    /* Selection */
   };
   #endif

A port is then defined with this structure using a constant offset:

   #ifdef __msp430_have_port1
   __MSP430_EXTERN__ struct port_full_t port1 asm("0x0020");
   #endif

The problem arises in that the second member ("out") of the port structure now has an offset of 0x0022 since the ioregister_t type is 2 bytes rather than its expected offset of 0x0021. The result for me was that code which used these structures to reference "out" were now pointing to what would have been "dir" instead.

This of course has nothing to do with my assertion that bit fields were at one time a mechanism to effect single instruction RMW cycles. You have certainly made me doubt that it was ever so. I now wonder if I am cross coupled in my mind with the TI paper on its TMS320 series processors "Programming TMS320x28xx and 28xxx Peripherals in C/C++ (SPRAA85)" which goes on about the advantages of bit field mapped registers and states:

   /When writing to a bit field, the compiler generates what is called
   a read-modify-write assembly
   instruction. Read-modify-write refers to the technique used to
   implement bit-wise or byte-wise
   operations such as AND, OR and XOR. That is, the location is read,
   the single bit field is modified, and
   the result is written back./

That is both a different processor and a different compiler but it is a case in which TI sanctioned the use of the bit field mapped register sets and it was my recollection (which I now doubt) that this was also applied to the MSP430 and then maybe only with code composer studio or the like.

So to be clear... I never meant that bit fields OUGHT to be used to implement single instruction RMW cycles. All that was understood was that IF you used the pre-defined bit fields then the compiler could be relied upon to generate a single instruction RMW. That may well have been due to the fact that such definitions were always volatile rather than to do with them being bit fields per se. Which leads me to something else you said:

   (There was a horribly grotesque hack intended
   to ensure RMW operations on volatiles got translated to single
   instructions, but it had nothing to do with bitfields and was
   demonstrably broken with complex expressions.  I replaced it with a
   different solution that has not yet been proven to do anything
   incorrect.)

Do you mean that you replaced it with a different solution which also translates RMW operations to single instructions or with a different solution which may or may not?

Also: I really do appreciate the your time responding to this. I have a lot of respect for all of you who maintain this compiler.


On 7/23/2012 3:24 PM, Peter Bigot wrote:
On Mon, Jul 23, 2012 at 2:49 PM, Paul Voith <p...@voithconsulting.com> wrote:
Peter,

I agree that bitfields are a pain. The standards guarantee almost nothing
regarding their internal implementation and so there is really no obvious
reason to implement what amounts to a hack using them.

While use of the a |= b may generate a single instruction IF b is a
constant, it is not guaranteed and in cases where b is an arbitrary
expression typically does not. The left side value is read; the expression
on the right calculated, the already read left side expression is OR'd in
and the the result written. Entering the expression for the right side into
a variable prior to ORing is likely to be removed by the compiler
optimization so nothing to rely on.
Yes.  I'm confused because, as you note in the previous paragraph,
there's nothing guaranteed about the behavior of bitfields either.  So
I'm not seeing what's changed here.  As I explain below, I see no
evidence mspgcc ever did anything special here.

Presently I notice that the 4.6.3 version of the compiler I am using accepts
bitfield definitions without generating an error. For bitfields defined on a
char or unsigned char base type it appears that bits so defined are packed
into a 16 bit short word. This is a change from early versions but appears
to have occurred well before 2008. This breaks the earlier include files
which used to define these as bit fields within an unsigned char object. So
legacy code compiles without error but generates an unexpected result. So
even if TI provided such files the present bitfield implementation could not
use them.
I believe there are situations where you'd need to add the attribute
"packed" to the structure declaration.  "packed" is a standard gcc
attribute and is documented in the gcc manual.  In other situations
that would not be necessary.

Since the C/C++ standards (and most HLLs) generally presume that memory does
not change value by itself and that reading from a memory location has no
consequence, I/O register access is problematic. The volatile modifier
allows handling of the first case of course by demanding that every explicit
reference to an identifier in an expression forces an actual read of the
associated memory location. [Aside: I wondered if it is allowed to read such
a location MORE times than explicitly referenced?].
With volatile?  No, it's not allowed.

Rather than trying to hack up the compiler it is probably best to just
generate an inline assembly instruction sequence which performs the needed
functionality.

I use msp430-gcc a lot in my work. I have actually been using an extremely
old version of the compiler and discovered the change when I recompiled code
(without error) which simply didn't work as before.
Can you provide a concrete example?

I can say that I
consider this ability to fiddle bits without semaphore protection of every
register a huge convenience.
Sure.  But there's nothing about bitfields that makes that more
likely, and some things about them that makes it less likely.

In my maintenance and evolution of the mspgcc port I haven't seen any
indication that mspgcc ever did anything to enable what you have come
to expect.  Certainly there were no traces left by the time mspgcc4
came into the picture.  (There was a horribly grotesque hack intended
to ensure RMW operations on volatiles got translated to single
instructions, but it had nothing to do with bitfields and was
demonstrably broken with complex expressions.  I replaced it with a
different solution that has not yet been proven to do anything
incorrect.)

In short, I can't see why you think the bitfield structures ever
provided the promises you've described.

With the current version of the compiler, things work pretty reliably
using standard C operations on standard data types, and I've not
encountered any situation where a semaphore protecting a single
register access was necessary.  If you have found situations where
things don't work, please file bug reports.

Also (and of less concern actually): even though a case has been made that
bitfields do not result in efficient code I think it is more that it lends
itself to inefficient program design. When I write |= and &= versions of
code which duplicate bitfield behavior they result in almost identical code
IF you make the code do identical things. So X.bit3 = Y  is really X |= (Y &
1) << 3 for example. IF Y is a constant both will result in a single
instruction.
Yes.  gcc translates all bitfield operations into sequences of shifts
and bitwise operations on values in the native word size in the middle
end, long before the target gets a chance to do anything.  At best
they're a convenience to users, and at worst introduce extra steps the
compiler has to work around (and sometimes fails to do so).

Peter

On 7/23/2012 5:15 AM, Peter Bigot wrote:
While that may or may not have been the understanding of users of mspgcc
during its earlier years, from my experience with GCC's internals any such
an assumption was a mistake. Today, you are far more likely to have "atomic"
RMW instructions if you use the standard C operators on natural integral
types than if you use bitfields. Bitfields are complex to implement
correctly, and gcc introduces overhead during analysis and optimization to
ensure it doesn't violate the language semantics at a stage when it isn't
concerned with target-specific capabilities. Google for "gcc bitfield"
reveals many examples where this produces hideous code on a variety of
targets.

In code with any reasonable level of optimization (-O, -Os), mspgcc should
generate single instruction implementations for & and | on standard C types
wherever supported by the underlying ISA. When it does not, this is a bug
that should be reported so it can be fixed. I believe other compilers
(llvm?) might make other promises, or provide an alternative solution (such
as intrinsics that are guaranteed to produce bic and bis instructions). I
would entertain a proposal to add such intrinsics. Bitfield structures will
be added only if the definitions are provided by TI in the headers shared
among all MSP430 compilers.

Peter


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Mspgcc-users mailing list
Mspgcc-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mspgcc-users

Reply via email to