arduino / arduino/ArduinoCore-samd

compiler defaults to -signed-enums

Open
#319 0 comments 1 reaction 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.