Benchmarking #526
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| #pragma once | ||
|
|
||
| #include <array> | ||
| #include <inttypes.h> | ||
| #include <algorithm> | ||
| #include <string_view> | ||
|
|
||
| #include "stm32h723xx.h" | ||
| #include "core_cm7.h" | ||
| #include "SEGGER_RTT.h" | ||
|
|
||
| enum class BenchmarkType : uint8_t{ | ||
| NONE, | ||
| RUNTIME, | ||
| GDB | ||
| }; | ||
| enum BenchmarkLoggingTypes{ | ||
| _NONE, | ||
| _SD = 1, | ||
| _RTT = 2 | ||
| }; | ||
|
|
||
| struct TimeStamp{ | ||
| uint32_t id; | ||
| uint32_t cycles; | ||
| }; | ||
|
|
||
|
|
||
| static inline TimeStamp timestamp; | ||
| /** | ||
| * This hash is used to send a lightweight identifier via RTT | ||
| * on the host the source code is searched for BENCHMARK_MACROS | ||
| * and hashed using the same hash algorithm, thus creating a 1:1 | ||
| * match between hash and string | ||
| */ | ||
| consteval uint32_t custom_hash(std::string_view str){ | ||
| uint32_t hash = 2166136261u; // FNV offset basis | ||
|
|
||
| for (char c : str) { | ||
| hash ^= (unsigned char)c; | ||
| hash *= 16777619u; // FNV prime | ||
| } | ||
|
|
||
| return hash; | ||
| } | ||
| #if defined(RUNTIME_BENCHMARK) && defined(GDB_BENCHMARK) | ||
| #error "Cannot use RUNTIME_BENCHMARK and GDB_BENCHMARK SIMULTANEOUSLY" | ||
| #endif | ||
|
|
||
| extern "C"{ | ||
| #ifdef RUNTIME_BENCHMARK | ||
| constexpr BenchmarkType benchmark_type = BenchmarkType::RUNTIME; | ||
| static const char BENCHMARK_TYPE_RUNTIME[] __attribute__((used)) = "BENCHMARK_RUNTIME"; | ||
| #elif defined(GDB_BENCHMARK) | ||
| constexpr BenchmarkType benchmark_type = BenchmarkType::GDB; | ||
| static const char BENCHMARK_TYPE_GDB[] __attribute__((used)) = "BENCHMARK_GDB"; | ||
| #else | ||
| constexpr BenchmarkType benchmark_type = BenchmarkType::NONE; | ||
| static const char BENCHMARK_TYPE_NONE[] __attribute__((used)) = "BENCHMARK_NONE"; | ||
| #endif | ||
|
|
||
| } | ||
|
|
||
| #if defined(RUNTIME_BENCHMARK) | ||
| #define RUNTIME_BENCHMARK_TAG _RTT | ||
| #else | ||
| #define RUNTIME_BENCHMARK_TAG _NONE | ||
| #endif | ||
|
|
||
| #if defined(SD) | ||
| #define SD_BENCHMARK_TAG _SD | ||
| #else | ||
| #define SD_BENCHMARK_TAG _NONE | ||
| #endif | ||
| inline constexpr int logging_type = RUNTIME_BENCHMARK_TAG | SD_BENCHMARK_TAG; | ||
| template<BenchmarkType Type> | ||
| class Benchmarker{ | ||
| public: | ||
| Benchmarker(){ | ||
| DWT->CTRL |= 1; | ||
| DWT->CYCCNT = 0; | ||
| if constexpr(benchmark_type == BenchmarkType::RUNTIME){ | ||
| SEGGER_RTT_Init(); | ||
| } | ||
| } | ||
| template<uint32_t Id> | ||
| __attribute__((always_inline)) inline static void __benchmark_start__(){ | ||
| if constexpr(benchmark_type == BenchmarkType::GDB){ | ||
| __asm__ volatile("BKPT \n"); | ||
| } else if constexpr(benchmark_type == BenchmarkType::RUNTIME){ | ||
| uint32_t cycles = DWT->CYCCNT; | ||
| if constexpr( logging_type & _RTT){ | ||
| timestamp.id = Id; | ||
| timestamp.cycles = cycles; | ||
| SEGGER_RTT_Write(0,×tamp,sizeof(TimeStamp)); | ||
| } | ||
| } | ||
| } | ||
| template<uint32_t Id> | ||
| __attribute__((always_inline)) inline static void __benchmark_end__(){ | ||
| if constexpr(benchmark_type == BenchmarkType::GDB){ | ||
| __asm__ volatile("BKPT \n"); | ||
| } else if constexpr(benchmark_type == BenchmarkType::RUNTIME){ | ||
| uint32_t cycles = DWT->CYCCNT; | ||
| if constexpr( logging_type & _RTT){ | ||
| timestamp.id = Id; | ||
| timestamp.cycles = cycles; | ||
| SEGGER_RTT_Write(0,×tamp,sizeof(TimeStamp)); | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+86
to
+111
There was a problem hiding this comment. The benchmark_start and benchmark_end functions read DWT->CYCCNT without any mechanism to handle or account for counter overflows. The num_overflows variable is declared in DataWatchpointTrace but never incremented. When the 32-bit counter wraps around, timing measurements will be incorrect. Consider implementing overflow detection and handling.
Member
Author
There was a problem hiding this comment. The overflow logic is implemented on the host side, to make the target measuremetns as lightweight as possible |
||
| }; | ||
|
|
||
| #define BENCHMARK_SETUP() \ | ||
| Benchmarker<benchmark_type> __b; | ||
| #define BENCHMARK_START(NAME) \ | ||
| Benchmarker<benchmark_type>::__benchmark_start__<custom_hash(NAME)>(); | ||
| #define BENCHMARK_END(NAME)\ | ||
| Benchmarker<benchmark_type>::__benchmark_end__<custom_hash(NAME)>(); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |||||
| #include "ErrorHandler/ErrorHandler.hpp" | ||||||
| #include "HALAL/Models/PinModel/Pin.hpp" | ||||||
| #include "core_cm7.h" | ||||||
| #include "HALAL/Benchmarking_toolkit/Benchmarking.hpp" | ||||||
| #if !defined DWT_LSR_Present_Msk | ||||||
| #define DWT_LSR_Present_Msk ITM_LSR_Present_Msk | ||||||
| #endif | ||||||
|
|
@@ -14,6 +14,8 @@ | |||||
| #define DWT_LAR_KEY 0xC5ACCE55 | ||||||
| #define DEMCR_TRCENA 0x01000000 | ||||||
| #define DWT_CTRL_CYCCNTENA 0x00000001 | ||||||
|
|
||||||
|
|
||||||
| /* | ||||||
| This class is designed to study the efficiency of an algorithm | ||||||
| by counting the number of clock cycles required for execution. | ||||||
|
|
@@ -28,36 +30,24 @@ | |||||
| */ | ||||||
| class DataWatchpointTrace { | ||||||
| public: | ||||||
| static uint32_t num_overflows; | ||||||
|
There was a problem hiding this comment. The static member variable num_overflows is declared but never defined or initialized. This will cause a linker error. Add a definition in the corresponding .cpp file (e.g., uint32_t DataWatchpointTrace::num_overflows = 0;) or make it inline if using C++17 or later.
Suggested change
|
||||||
| static void enable() { | ||||||
| DWT->CTRL |= DWT_CTRL_CYCCNTENA | DWT_CTRL_EXCTRCENA_Msk | DWT_CTRL_EXCEVTENA_Msk; // enables the counter | ||||||
| DWT->FUNCTION0 |= (1 << 7) | 0b1000; // enable comparison | ||||||
| DWT->COMP0 = 0xFFFFFFFF; // detect overflow | ||||||
| } | ||||||
|
Comment on lines
+34
to
38
There was a problem hiding this comment. The enable() function modifies DWT hardware registers but does not verify that CoreDebug->DEMCR has the TRCENA bit set, which is required before accessing DWT registers. This could cause the DWT operations to fail silently. Consider adding CoreDebug->DEMCR |= DEMCR_TRCENA before accessing DWT registers. |
||||||
|
|
||||||
| static void disable(){ | ||||||
| DWT->CTRL &= ~DWT_CTRL_CYCCNTENA; // disable the counter | ||||||
| } | ||||||
|
|
||||||
| static unsigned int get_count() { | ||||||
| return DWT->CYCCNT; // returns the current value of the counter | ||||||
| } | ||||||
|
|
||||||
| private: | ||||||
| static void reset_cnt() { | ||||||
| DWT->CYCCNT = 0; // reset the counter | ||||||
|
|
||||||
| } | ||||||
|
Comment on lines
49
to
52
There was a problem hiding this comment. The reset_cnt() private method is declared but has an empty function body, and it no longer performs its intended reset operation. This appears to be dead code that should either be removed or properly implemented to reset the counter as its name suggests. |
||||||
| }; | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Benchmarker constructor initializes DWT registers (DWT->CTRL and DWT->CYCCNT) without checking if the trace infrastructure is unlocked or enabled via CoreDebug->DEMCR. This could fail silently on some targets. The initialization should call CoreDebug->DEMCR |= DEMCR_TRCENA first, and potentially unlock DWT if needed.