Handling disjoint arrays

[nathanPPS]

I've run into an svd file where a set of registers is allocated linearly in memory, but only specific registers can be used. For example the span 0-31 is allocated for a register type, but only 0-3, 8, 9, 12, 14, 15 and 18 can be used. See ELSR* registers in R7FA2E1A9.svd from the Renesas RA DFP. When parsed by svd2rust version 0.25.1, this creates new types for each register grouping, often causing name collisions later when compiling the crate. I can see two ways to handle this:

1. Create a continuous array including the dead addresses. This makes for a simpler API but may lead to undefined behavior when trying to access the dead addresses.
2. Create one register for each live address with a unique name that shares the same type. This would be safer, but the API isn't quite as ergonomic to use.

I lean towards option two for the safety reasons, but am interested in other options for handling this use case.

[burrbull]

I don't have a clear opinion about this.
On the one hand this example looks incompatible with CMSYS SVD specification.
On the other hand I don't see correct way to describe such registers. In my view there should derivedFrom be used instead of array.

[n8tlarsen]

I looked through the CMSIS SVD specification and couldn't find anything saying the description used for the R7FA2E1A9.svd ELSR* registers is invalid. In fact running the SVD file through the CMSIS SVDConv tool generates the following C struct:

typedef struct {                            /*!< (@ 0x40041000) ELC Structure                                    */
    __IOM uint8_t   ELCR;                   /*!< (@ 0x00000000) Event Link Controller Register                   */
    __IM  uint8_t   RESERVED;
    __IOM uint8_t   ELSEGR0;                /*!< (@ 0x00000002) Event Link Software Event Generation Register 0  */
    __IM  uint8_t   RESERVED1;
    __IOM uint8_t   ELSEGR1;                /*!< (@ 0x00000004) Event Link Software Event Generation Register 1  */
    __IM  uint8_t   RESERVED2;
    __IM  uint16_t  RESERVED3[5];
    __IOM uint16_t  ELSR0;                  /*!< (@ 0x00000010) Event Link Setting Register 0                    */
    __IM  uint16_t  RESERVED4;
    __IOM uint16_t  ELSR1;                  /*!< (@ 0x00000014) Event Link Setting Register 1                    */
    __IM  uint16_t  RESERVED5;
    __IOM uint16_t  ELSR2;                  /*!< (@ 0x00000018) Event Link Setting Register 2                    */
    __IM  uint16_t  RESERVED6;
    __IOM uint16_t  ELSR3;                  /*!< (@ 0x0000001C) Event Link Setting Register 3                    */
    __IM  uint16_t  RESERVED7[9];
    __IOM uint16_t  ELSR8;                  /*!< (@ 0x00000030) Event Link Setting Register 8                    */
    __IM  uint16_t  RESERVED8;
    __IOM uint16_t  ELSR9;                  /*!< (@ 0x00000034) Event Link Setting Register 9                    */
    __IM  uint16_t  RESERVED9[5];
    __IOM uint16_t  ELSR12;                 /*!< (@ 0x00000040) Event Link Setting Register 12                   */
    __IM  uint16_t  RESERVED10[3];
    __IOM uint16_t  ELSR14;                 /*!< (@ 0x00000048) Event Link Setting Register 14                   */
    __IM  uint16_t  RESERVED11;
    __IOM uint16_t  ELSR15;                 /*!< (@ 0x0000004C) Event Link Setting Register 15                   */
    __IM  uint16_t  RESERVED12[5];
    __IOM uint16_t  ELSR18;                 /*!< (@ 0x00000058) Event Link Setting Register 18                   */
} ELC_Type;                                   /*!< Size = 90 (0x5a)                                              */

Which takes the same approach as option 2 above, albeit without svd2rust's typing of registers and fields. Granted in this example the array would have to be expanded anyway because the the register size is smaller than the dimIncrement. Nevertheless, the proposed feature now applies a common type to each ELSR* register.

My opinion is usually to let my tools do the most work they can. In this case the alternative would be to manually go back and patch each ELSR* register present with derivedFrom. Or I can have the tool infer this for me. The output should be the same, but letting the tool do the work for me saves me time. What do you think @burrbull?

[burrbull]

How SVDConv would convert this block if register size equal to dimIncrement? Does it use C arrays? Would it use C arrays for ELSR0-3?

[n8tlarsen]

Strangely enough, after looking through a few other examples SVDConv doesn't seem to implement C arrays at all for registers specified as such in the SVD (even though I would have expected it to do so in a few cases I looked at). Regardless, I think the main point is the SVD file is valid according to SVDConv which checks against the specification before generating C headers. That being the case, I think the svd2rust tool should make an effort to do something intelligent with a valid SVD, even if that SVD would be better written using derivedFrom. Would you agree?

[burrbull]

I'm not opposite to support this, specifically option 2.
I just afraid of big growing of codebase. Even now it is not easy to support it.

[n8tlarsen]

In total this feature adds about 350 lines of code which is mostly contained to a single function check_erc_reprs. I work in embedded systems professionally and would love to use Rust more, but many parts don't yet have PAC support and this feature would help provide less barrier to entry for more parts, specifically all parts from the Renesas RA series. I understand code bloat should be avoided, but I don't think this falls into that category.

[burrbull]

I'll leave this open until CI tests and register contents compare be implemented.