Hi Thomas,
The first variable in a tfm_core_irq_signal_data_t structure is a partition_id. ID 0 is reserved and cannot be a valid entry in this array so it could, in theory, be checked for terminating the loop. I would argue, however, that having a compile time constant loop count variable is better from a performance and optimization point of view. If loop count is const 0 an optimizing compiler can eliminate the loop altogether while if the iteration count depends on the value of an entry in the array, it would be more unlikely to eliminate the unused loop.
The key metrics that TF-M is currently measured against are memory footprint and to a lesser degree latency, and having the extra element in the array and the extra iteration in the loop are both sub-optimal in this respect. I do understand the need, however, to keep the constructs standard C and eliminate deviations. Those can lead to build errors or worse: unexpected runtime behaviour with a simple change in a compiler version or similar, so having a clean, standard, predictable code is essential. So thank you for your effort and yes, until and unless we find a more optimal way to get around this problem, please do introduce the proposed change, taking into possible consideration my first comment on helping the compiler detect the unused code.
Regards Miklos
-----Original Message----- From: TF-M tf-m-bounces@lists.trustedfirmware.org On Behalf Of Thomas Törnblom via TF-M Sent: 14 August 2019 08:53 To: tf-m@lists.trustedfirmware.org Subject: [TF-M] tfm_core_irq_signals array
Just back from vacation and I notice that a lot seems to have happened while I was away :)
As you may know, I'm adding support for the IAR toolchain to tf-m, and this mostly involve cleaning up and eliminating non-standard C constructs that gcc and armclang seems to accept, but which our toolchain does not and is non-standard.
One thing we don't accept is zero sized arrays, and I cleaned up a few instances before going on vacation and now I see that a new one has crept in, tfm_core_irq_signals.
For the other instances I added a zero initialized element at the end of the array and instead of generating a "count" variable at compile time I used that trailing zero element as a limit while looping over the array. In those cases it was fairly easy and safe as there was at least one pointer element that was non-null for all elements except the last. In this case all members of the struct are scalars and I'm not sure if there is a value I can assign to any of the members that are guaranteed to never be legal and that I can test for.
Or should I just use the changes below, where I add a dummy trailing element and adjust the tfm_core_irq_signals_count variable for this?
--- /* Definitions of the signals of the IRQs */ const struct tfm_core_irq_signal_data_t tfm_core_irq_signals[] = { #ifdef TFM_PARTITION_TEST_CORE { TFM_IRQ_TEST_1_ID, SPM_CORE_IRQ_TEST_1_SIGNAL_TIMER_0_IRQ, TFM_TIMER0_IRQ, 64 }, #endif /* TFM_PARTITION_TEST_CORE */ {0, 0, 0, 0} };
const size_t tfm_core_irq_signals_count = (sizeof(tfm_core_irq_signals) / sizeof(*tfm_core_irq_signals)) - 1; ---
/Thomas