2

Used architecture: Xtensa-LX6

I have a following definition:

volatile typedef struct{
    union{
        struct{
            uint32_t FIELD1:16;
            uint32_t FIELD2:12;
            uint32_t FIELD3:4;
        };
        uint32_t _;
    }REGISTER_1; //0x00 offset, 4bytes width
}registers_t;
static registers_t* HARDWARE = (registers_t *)0x3FF43000;

Then I have a line somewhere in the code:

HARDWARE->REGISTER_1.FIELD1 = 0b10101u;

This line gets compiled by xtensa gcc to the following assembly code (beautified):

HARDWARE:
    .word   1073102848

THE_LINE_OF_CODE:
    .literal .LC120, HARDWARE ; .LC120 now holds the address of HARDWARE - 0x3FF43000
    .align  4                 ; assembler directive
    l32r    a2, .LC120        ; reg_a2 <= HARDWARE
    l32i.n  a2, a2, 0         ; reg_a2 <= memory_at(reg_a2) //reg_2a <= 0x3FF43000
    movi.n  a3, 0b10101       ; reg_a3 <= 0b10101
    memw                      ; wait for memory operations to finish
    s16i    a3, a2, 0         ; memory_at(reg_a2+0) <= a3 // *(0x3FF43000) = 0b10101

As you can see, assembler absolutely ignored the rest of the register fields, it just assumes the lower 16 bits will be written to, and writes the 16 bit value there. The problem is that the registers in xtensa architecture are always 4-bit wide and aligned so. Thus if we write the register content in binary form: REGISTER1: 0b----------------1111111111111111, the '-' portion just gets deleted, not retaining its value. The register gets corrupted and an avalanche of errors happens.

This can be avoided by inserting __attribute__((packed)) into the struct declaration:

    union{
        struct{
            uint32_t FIELD1:16;
            uint32_t FIELD2:12;
            uint32_t FIELD:4;
        }__attribute__((packed));
        uint32_t _;
    }REGISTER_1; //0x00 offset, 4bytes width

Now the assembler knows what we are looking for, and the resulting assembly is (beautified):

HARDWARE:
    .word   1073102848

THE_LINE_OF_CODE:
    .literal .LC120, HARDWARE ; .LC120 now holds the address of HARDWARE - 0x3FF43000
    .literal .LC121, 0xFFFF0000 ; A bit mask for our portion
    .align  4                 ; assembler directive
    l32r    a2, .LC120        ; a2 <= HARDWARE
    l32i.n  a2, a2, 0         ; a2 <= memory_at(a2) //2a <= 0x3FF43000
    memw                      ; wait for memory operations to finish
    l32i.n  a4, a2, 0         ; a4 <= memory_at(a2 + 0 the struct offset)
    l32r    a3, .LC121        ; a3 <= 0xFFFF0000
    and a4, a4, a3            ; a4 <= a4 & a3 (zeroes the lower 16 bits we will be writing to, preserves rest)
    movi.n  a3, 0b10101       ; a3 <= 0b10101
    or  a3, a4, a3            ; a3 <= a4 | a3 (writes to the lower 16 bits)
    memw                      ; wait for memory operations to finish
    s32i.n  a3, a2, 0         ; memory_at(a2 + 0 the struct offset) <= a3

Problem solved! Great. But..

THE REAL QUESTION STARTS HERE:

I can have many structures in my registers definition:

volatile typedef struct{
    union{
        struct{...}__attribute__((packed));
        uint32_t _;
    }REGISTER_1;
    union{
        struct{...};
        uint32_t _;
    }REGISTER_2;
    .
    .
    .
}registers_t;
static registers_t* HARDWARE = (registers_t *)0x3FF43000;

How to pack the structures so that the alignment in the whole typedef is preserved, and each bitfield operation is compiled correctly? Putting __attribute__((packed)) everywhere may work, but it breaks alignment - the overall sizeof the typedef is larger than demanded.

Patrik Staron
  • 331
  • 1
  • 9
  • *How to pack the structures so that the alignment in the whole typedef is preserved, and each bitfield operation is compiled correctly?* Don't use bitfields? [They are entirely non-portable](https://port70.net/~nsz/c/c11/n1570.html#6.7.2.1p11): "... whether a bit-field that does not fit is put into the next unit or overlaps adjacent units is implementation-defined. The order of allocation of bit-fields within a unit (high-order to low-order or low-order to high-order) is implementation-defined. The alignment of the addressable storage unit is unspecified." – Andrew Henle Apr 04 '21 at 17:09
  • @Andrew Henle thank you for a comment. I know this is not an ideal way how to do it, but the code is very clean and program may be well optimized on the instruction level because the compiler is aware of what a programmer wants to do. There is just this disadvantage of alignment. My goal here would be to patch it up the best way possible. – Patrik Staron Apr 04 '21 at 17:18
  • 2
    Can you hack around it with `alignas(4)` on the packed structs, or on the unions that contain them? Also `HARDWARE` should be `static spi_t* const HARDWARE` to help the compiler inline that pointer constant in code that uses it. Although being static, it hopefully notices that nothing writes that variable so it doesn't have to load a pointer from static storage before deref, even without const. – Peter Cordes Apr 04 '21 at 17:23
  • improper use of unions, pointing structs across compile domains, etc are always going to be problematic, best to avoid them like the plague, They will always fail...eventually... – old_timer Apr 04 '21 at 17:43
  • 1
    Structure and bitfields packing is always implementation-specific. You need to read the documentation for your specific C compiler as to how to pack things properly. This will in general only work for that specific C compiler and may even fail on future versions of the same C compiler (though hopefully the documentation will give you some hints on what future updates are likely to preserve). – Chris Dodd Apr 04 '21 at 19:30

0 Answers0