arduino / arduino/ArduinoCore-samd
compiler defaults to -signed-enums
- Dominant language
- C
- Stars
- 502
- Forks
- 740
- PR merge metrics
- No merged PRs in 30d
Description
Despite the hungarian notation indicating the various fields in g_APinDescription[] are unsigned longs, several of them are actually enums, and the ARM gcc seems to default to making them as short as possible, and examination of the binary produced for SAMD21 shows that "many" end up as int8_t or int16_t
```
typedef struct _PinDescription
{
EPortType ulPort ;
uint32_t ulPin ;
EPioType ulPinType ;
uint32_t ulPinAttribute ;
EAnalogChannel ulADCChannelNumber ; /* ADC Channel number in the SAM device */
EPWMChannel ulPWMChannel ;
ETCChannel ulTCChannel ;
EExt_Interrupts ulExtInt ;
}
```
```
(gdb) print sizeof(g_APinDescription)
$1 = 1152
(gdb) print sizeof(g_APinDescription[1])
$2 = 24
(gdb) print sizeof(g_APinDescription[1].ulPort)
$3 = 1
(gdb) print sizeof(g_APinDescription[1].ulPin)
$4 = 4
(gdb) print sizeof(g_APinDescription[1].ulPinType)
$5 = 1
(gdb) print sizeof(g_APinDescription[1].ulPinAttribute)
$6 = 4
(gdb) print sizeof(g_APinDescription[1].ulADCChannelNumber)
$7 = 1
(gdb) print sizeof(g_APinDescription[1].ulPWMChannel)
$8 = 2
(gdb) print sizeof(g_APinDescription[1].ulTCChannel)
$9 = 2
(gdb) print sizeof(g_APinDescription[1].ulExtInt)
$10 = 1
```
This has two detrimental conflicting effects.
1) The fields are not arranged for efficient packing of the data structure. It could be 1/3 smaller just by re-arranging.
2) since the enums also end up signed, the ARM compiler produces "ldrsb" (load signed byte) instructions to fetch the value. Unfortunately, ldrsb has some restrictions on the addressing modes it can use (on CM0), so it ends up using an extra instruction and a register that wouldn't be necessary if it were actually an int32:
```
if ( g_APinDescription[ulPin].ulPinType == PIO_NOT_A_PIN )
462e: 2408 movs r4, #8
4630: 5714 ldrsb r4, [r2, r4]
```
vs
```
if ( g_APinDescription[ulPin].ulPinType == PIO_NOT_A_PIN )
8: 6894 ldr r4, [r2, #8]
```
All things considered, this is probably insignificant, but since I bothered to chase it down, I thought I'd document it as well
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by inspecting the _PinDescription declaration and g_APinDescription for the enum field sizes, then reproduce the ARM gcc/SAMD21 sizeof and disassembly observations shown. The issue does not define a resolution or acceptance criteria; clarify whether the goal is field reordering, enum-width changes, or documentation before implementing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- embedded-iot
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100