From 4c712c1f2cb88f63667c279239ff382438c2c116 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 3 Feb 2023 10:05:22 +0100 Subject: [PATCH] [MOD] Remove constexpr usage This was introduced when STM32WL support was added. Using constexpr for the END_OF_MODE_TABLE constant allows it to be initialized in the class declaration, but this needs C++11. This moves the initialization out of the class declaration to the .cpp file, which does not need constexpr. It also lets STM32WLx::END_OF_MODE_TABLE define its value directly (instead of aliasing Module::END_OF_MODE_TABLE) to prevent reduce runtime overhead (see below). The downside of this change is that the value of the END_OF_MODE_TABLE is no longer visible in other compilation units and thus cannot be inlined into the rfswitch_table (if used). For example, on STM32, this means that instead of having a pre-cooked rfswitch_table that lives in the .rodata section (so can be read directly from flash), the table lives in RAM and is initialized at runtime (the actual modes and pins are copied from flash to RAM by the standard startup loop that copies all of the .data section, and the END_OF_MODE_TABLE value is copied by a bit of new generated code). This means a little more runtime overhead, but the cost is mostly in RAM size (80 bytes for the SMT32WL sketches, 16 per mode plus 16 for the END_OF_MODE_TABLE). In a first attempt at this commit, STM32WLx::END_OF_MODE_TABLE was still initialized using the Module::END_OF_MODE_TABLE value, but since the latter is also not available at compiletime, this meant initialization of the former also needed to happen at runtime, adding even more code overhead (and possibly leading to ordering issues as well). To avoid this, the STM32WLx::END_OF_MODE_TABLE initialization now just duplicates that of Module::END_OF_MODE_TABLE. On AVR, the impact is not so much: Since AVR cannot address flash directly, the table was already copied from flash to RAM at startup, so the extra RAM usage is just 4 bytes because END_OF_MODE_TABLE is now also present in RAM, to be copied into rfswitch_table at startup. Options for avoiding this overhead (not implemented in this commit) could be (in no particular order): 1. Use a macro instead of a constant. Downside is that these cannot be scoped inside the Module/STM32WLx classes like now, so this requires changes to sketches that use a rfswitch_table (and reduced scoping and using macros adds more opportunity for conflicts and weird errors). 2. Apply the change in this commit only when C++11 is not available. Downside is that the initialization value of these constants must be duplicated in the .h and .cpp file for C++ and older versions respectively. 3. Let sketches just use `{Module::MODE_END_OF_TABLE, {}}` explicitly instead of `Module::END_OF_MODE_TABLE`. Downside of this is that this requires sketches to be modified and that it lets the sketch encode more of the table structure, potentially making future API changes harder (but it probably does not really matter in practice). 4. Turn END_OF_MODE_TABLE into a static method, which *can* then be defined in the class declaration and inlined. The method can then be conditionally marked as constexpr, which allows C++11 compilers to completely resolve the rfswitch_table value at compiletime, producing a binary identical to before this commit. When constexpr is omitted (e.g. on older compilers), some runtime overhead is added (pretty much the same as the result from this commit). Downside is that sketches must be modified, and the `END_OF_MODE_TABLE` "constant" must now be called, e.g. `END_OF_MODE_TABLE()` which might be a bit unexpected syntax. --- src/Module.cpp | 2 ++ src/Module.h | 2 +- src/modules/SX126x/STM32WLx.cpp | 2 ++ src/modules/SX126x/STM32WLx.h | 2 +- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Module.cpp b/src/Module.cpp index 5046da63..522d6ae2 100644 --- a/src/Module.cpp +++ b/src/Module.cpp @@ -8,6 +8,8 @@ mbed::PwmOut *pwmPin = NULL; #endif +const Module::RfSwitchMode_t Module::END_OF_MODE_TABLE = {Module::MODE_END_OF_TABLE, {}}; + Module::Module(RADIOLIB_PIN_TYPE cs, RADIOLIB_PIN_TYPE irq, RADIOLIB_PIN_TYPE rst, RADIOLIB_PIN_TYPE gpio): _cs(cs), _irq(irq), diff --git a/src/Module.h b/src/Module.h index 78793d0a..526f24fa 100644 --- a/src/Module.h +++ b/src/Module.h @@ -62,7 +62,7 @@ class Module { * * See setRfSwitchTable() for details. */ - static constexpr RfSwitchMode_t END_OF_MODE_TABLE = {MODE_END_OF_TABLE, {}}; + static const RfSwitchMode_t END_OF_MODE_TABLE; #if defined(RADIOLIB_BUILD_ARDUINO) diff --git a/src/modules/SX126x/STM32WLx.cpp b/src/modules/SX126x/STM32WLx.cpp index 744f88c3..6897dfc1 100644 --- a/src/modules/SX126x/STM32WLx.cpp +++ b/src/modules/SX126x/STM32WLx.cpp @@ -11,6 +11,8 @@ This file is licensed under the MIT License: https://opensource.org/licenses/MIT #include +const Module::RfSwitchMode_t STM32WLx::END_OF_MODE_TABLE = {Module::MODE_END_OF_TABLE, {}}; + STM32WLx::STM32WLx(STM32WLx_Module* mod) : SX1262(mod) { } diff --git a/src/modules/SX126x/STM32WLx.h b/src/modules/SX126x/STM32WLx.h index 5778dfb1..ad7faffb 100644 --- a/src/modules/SX126x/STM32WLx.h +++ b/src/modules/SX126x/STM32WLx.h @@ -63,7 +63,7 @@ class STM32WLx : public SX1262 { MODE_TX_HP, }; /*! \copydoc Module::END_OF_MODE_TABLE */ - static constexpr auto END_OF_MODE_TABLE = Module::END_OF_MODE_TABLE; + static const Module::RfSwitchMode_t END_OF_MODE_TABLE; // basic methods