5

I'm programming C on an embedded system. The processor architecture is 32 bits (sizeof(int) is 32 bits, sizeof(short) is 16 bits). There is a 32-bit variable that is a memory-mapped control register (CTRL_REG) that is specified as only the bottom 16 bits being used, and they contain a signed 16 bit integer value (writing to the higher bits has no effect). Memory accesses must be 32-bit aligned, so I can't just bump the pointer over a couple bytes, and also I cannot assume an endianness. I'm concerned that automatic type promotion is going to mess with whatever I'm storing by extending the sign bit out to bit 31 instead of leaving it at bit 15 where I want it. What is the best way to store something in this location?

Here was my original code, which I am nearly certain is wrong:

#define CTRL_REG   *((volatile unsigned int *)0x4000D008u)
short calibrationValue;

CTRL_REG = -2 * calibrationValue;

Then I tried this, but I think it still might be subject to integer promotion at the point of the assignment:

CTRL_REG = (short)(-2 * calibrationValue);

Then finally I thought of this:

CTRL_REG = (unsigned short)(short)(-2 * calibrationValue);

I can't evaluate these options very well because they all work in my tests because calibrationValue happens to be negative (it's a calibration parameter specific to each device, and so could be positive on some devices), so after multiplying by -2, I end up storing a positive value and thus I don't actually run into the problem I'm expecting in my tests.

Your help is greatly appreciated, even if it's just to say "you're thinking too much".

Bo Persson
  • 90,663
  • 31
  • 146
  • 203
rmeador
  • 25,504
  • 18
  • 62
  • 103
  • If you want to be sure that your variable is 32 bits, you may want to use `uint32_t` instead, from stdint.h. Note that an `int` [doesn't have to be 32 bits](http://codinghorror.typepad.com/.a/6a0120a85dcdae970b0128776ff992970c-pi), not even on 32 bits systems. –  Sep 16 '11 at 14:35
  • 1
    @WTP: thanks for your concern. we have a compiler-specific header that contains type definitions that guarantee the size. They're not the standard uint32_t or whatever nomenclature, so rather than confuse my post with nonstandard types, I opted to convert everything to types everyone has seen before. – rmeador Sep 16 '11 at 14:40
  • 1
    I'd suggest at least one extra pair of parentheses around the macro: "(*((volatile unsigned int *)0x4000d008u))" – wildplasser Sep 16 '11 at 14:52
  • To determine endianness just write a word and then check one of the chars. For example `int i = 1; return *(char*)i == 1` will return true if the system is little endian – phuclv Oct 23 '13 at 08:53

4 Answers4

4

Instead of an unsigned int, define a union of an unsigned int and 2 short signed ints.

That's how I handle the 'funny hardware control registers' on my ARM systems where, quite often, odd bits here and there are either 'don't care' or 'must not have 1 written to them'.

Rgds, Martin

Martin James
  • 24,453
  • 3
  • 36
  • 60
4

Think about what a -16 ( for example ) looks like in 16 bit : '0xFFF0' , and in 33 bit: '0xFFFFFFF0'. Sign extending is exactly what you want to ensure you have a sign bit in the right place. Since the upper 16 are don't-care, it's fine to fill them with 1s. So create a signed 32 bit value, then cast it to an unsigned 32 to put in your register: 

  Int32 calreg= -2L * calibrationValue;
   CTRl_REG = (Uint32)calreg;

If you would rather write 0s to the high bits, mask with 0xFFFF before the cast.

AShelly
  • 34,686
  • 15
  • 91
  • 152
  • I think you've discovered the flaw in my reasoning... I was thinking that the sign bit got moved, rather than the entire intervening number of bits being filled with 1s. If I'm following correctly, even the intermediate variable and the cast in your example are unnecessary, and my original (simplest) code will work, regardless of how the compiler may extend/promote it. Is this correct? – rmeador Sep 16 '11 at 15:04
  • Yes, that's correct; the original simple code will work fine. – Brooks Moses Sep 16 '11 at 18:34
  • The original code should work. I would still add the cast, both to avoid compiler warnings and to make it clear that the change of type is intentional. I'm not sure the 'L' after the 2 is stictly needed, but I'd rather include it instead of finding out I didn't quite predict the integer promotion rules correctly: `CTRL_REG = (Uint32)(-2L * calibrationValue);` – AShelly Sep 16 '11 at 18:43
0

If the values written to the most significant 16 bits is ignored I think you can safely rely on type conversion. All in all, you don't care about the higher bits.

#define CTRL_REG   *((volatile signed int *)0x4000D008u)
short calibrationValue;

CTRL_REG = -2 * calibrationValue;

Notice that the CTRL_REG is declared as a signed integer now.

cyco130
  • 4,654
  • 25
  • 34
  • I'm afraid I don't have the ability to redefine that macro as signed. I don't think it matters though, as others have pointed out. – rmeador Sep 16 '11 at 15:05
0
#define SS2L(sh,sl) ( ((sh) <<16) | ((sl) & 0xffff) )

To combine two 16 bit shorts into one 32 bit long.

It is best for bitops to be done using unsigned types; promotion to int can grab you. Maybe add some extra casts to unsigned to the above macro, too.

wildplasser
  • 43,142
  • 8
  • 66
  • 109