From fc00236d7923478bda8112704edd209dd4189519 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Tue, 15 Oct 2019 18:01:05 -0300 Subject: [PATCH 1/5] components/esp_common: added esp_macros.h that aims to hold useful macros esp_common/esp_compiler: renamed esp_macros file to a more specific one esp_common/esp_compiler: removed CONTAINER_OF macro, it was a duplicate components/freertos: placed likely macros around port and critical sections component/freertos: placed likely macros on lists module components/freertos: placed unlikely macros inside of assertion points, they likely wont fail components/freertos: added likely macros on queue modules FreeRTOS queues are one of most hot code path, because to queues itself tend to be used a lot by the applications, besides that, queues are the basic primitive to form both mutexes and semaphores, The focus here is to place likely macros inside lowest level send and receive routines, since they're common from all kobjects: semaphores, queues, mutexes and FR internals (like timer queue) components/lwip: placed likely/unlikey on net-interfaces code components/fatfs: added unlikely macros on disk drivers code components/spiffs: added unlikely macros on low level fs driver components/freertos: added likely/unlikely macros on timers and ticker freertos/event_group: placed likely/unlikely macros on hot event group code paths components/sdmmc: placed likely / unlikely macros on lower level path of sdmmc components/bt: placed unlikely macros around bt HCI functions calling components/lwip: added likely/unlikely macros on OS port code section components/freertos: fix code style on tick handler --- .../host/nimble/esp-hci/src/esp_nimble_hci.c | 1 + components/esp_common/include/esp_compiler.h | 28 +++++++++++++++++++ components/fatfs/diskio/diskio_rawflash.c | 2 +- components/fatfs/diskio/diskio_rawflash.h | 1 + components/fatfs/diskio/diskio_sdmmc.c | 4 +-- components/fatfs/diskio/diskio_sdmmc.h | 1 + components/fatfs/diskio/diskio_wl.c | 6 ++-- components/fatfs/diskio/diskio_wl.h | 1 + components/freertos/event_groups.c | 2 +- .../freertos/include/freertos/FreeRTOS.h | 2 ++ .../include/freertos/FreeRTOSConfig.h | 4 +-- components/freertos/list.c | 3 +- components/freertos/port.c | 1 + components/freertos/queue.c | 10 +++---- components/freertos/tasks.c | 11 ++++---- components/freertos/timers.c | 6 ++-- .../lwip/port/esp32/freertos/sys_arch.c | 1 + components/lwip/port/esp32/netif/ethernetif.c | 7 +++-- components/lwip/port/esp32/netif/wlanif.c | 5 ++-- components/sdmmc/sdmmc_io.c | 13 +++++---- components/spiffs/spiffs_api.c | 4 +-- components/spiffs/spiffs_api.h | 1 + 22 files changed, 78 insertions(+), 36 deletions(-) create mode 100644 components/esp_common/include/esp_compiler.h diff --git a/components/bt/host/nimble/esp-hci/src/esp_nimble_hci.c b/components/bt/host/nimble/esp-hci/src/esp_nimble_hci.c index 0d1e4112f..0c0e9ce98 100644 --- a/components/bt/host/nimble/esp-hci/src/esp_nimble_hci.c +++ b/components/bt/host/nimble/esp-hci/src/esp_nimble_hci.c @@ -28,6 +28,7 @@ #include "esp_nimble_hci.h" #include "esp_bt.h" #include "freertos/semphr.h" +#include "esp_compiler.h" #define NIMBLE_VHCI_TIMEOUT_MS 2000 diff --git a/components/esp_common/include/esp_compiler.h b/components/esp_common/include/esp_compiler.h new file mode 100644 index 000000000..64ae87802 --- /dev/null +++ b/components/esp_common/include/esp_compiler.h @@ -0,0 +1,28 @@ +// Copyright 2016-2019 Espressif Systems (Shanghai) PTE LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +#ifndef __ESP_COMPILER_H +#define __ESP_COMPILER_H + +/* + * The likely and unlikely macro pairs: + * These macros are useful to place when application + * knows the majority ocurrence of a decision paths, + * placing one of these macros can hint the compiler + * to reorder instructions producing more optimized + * code. + */ +#define likely(x) __builtin_expect(!!(x), 1) +#define unlikely(x) __builtin_expect(!!(x), 0) + +#endif \ No newline at end of file diff --git a/components/fatfs/diskio/diskio_rawflash.c b/components/fatfs/diskio/diskio_rawflash.c index 363bd9b32..01fc05b90 100644 --- a/components/fatfs/diskio/diskio_rawflash.c +++ b/components/fatfs/diskio/diskio_rawflash.c @@ -40,7 +40,7 @@ DRESULT ff_raw_read (BYTE pdrv, BYTE *buff, DWORD sector, UINT count) const esp_partition_t* part = ff_raw_handles[pdrv]; assert(part); esp_err_t err = esp_partition_read(part, sector * SPI_FLASH_SEC_SIZE, buff, count * SPI_FLASH_SEC_SIZE); - if (err != ESP_OK) { + if (unlikely(err != ESP_OK)) { ESP_LOGE(TAG, "esp_partition_read failed (0x%x)", err); return RES_ERROR; } diff --git a/components/fatfs/diskio/diskio_rawflash.h b/components/fatfs/diskio/diskio_rawflash.h index 73ff15f88..48d2dd76e 100644 --- a/components/fatfs/diskio/diskio_rawflash.h +++ b/components/fatfs/diskio/diskio_rawflash.h @@ -20,6 +20,7 @@ extern "C" { #endif #include "esp_partition.h" +#include "esp_compiler.h" /** * Register spi flash partition diff --git a/components/fatfs/diskio/diskio_sdmmc.c b/components/fatfs/diskio/diskio_sdmmc.c index 115bd59e7..b1230f1a3 100644 --- a/components/fatfs/diskio/diskio_sdmmc.c +++ b/components/fatfs/diskio/diskio_sdmmc.c @@ -37,7 +37,7 @@ DRESULT ff_sdmmc_read (BYTE pdrv, BYTE* buff, DWORD sector, UINT count) sdmmc_card_t* card = s_cards[pdrv]; assert(card); esp_err_t err = sdmmc_read_sectors(card, buff, sector, count); - if (err != ESP_OK) { + if (unlikely(err != ESP_OK)) { ESP_LOGE(TAG, "sdmmc_read_blocks failed (%d)", err); return RES_ERROR; } @@ -49,7 +49,7 @@ DRESULT ff_sdmmc_write (BYTE pdrv, const BYTE* buff, DWORD sector, UINT count) sdmmc_card_t* card = s_cards[pdrv]; assert(card); esp_err_t err = sdmmc_write_sectors(card, buff, sector, count); - if (err != ESP_OK) { + if (unlikely(err != ESP_OK)) { ESP_LOGE(TAG, "sdmmc_write_blocks failed (%d)", err); return RES_ERROR; } diff --git a/components/fatfs/diskio/diskio_sdmmc.h b/components/fatfs/diskio/diskio_sdmmc.h index d7a50221b..6036ab215 100644 --- a/components/fatfs/diskio/diskio_sdmmc.h +++ b/components/fatfs/diskio/diskio_sdmmc.h @@ -16,6 +16,7 @@ #include "sdmmc_cmd.h" #include "driver/sdmmc_host.h" +#include "esp_compiler.h" #ifdef __cplusplus extern "C" { diff --git a/components/fatfs/diskio/diskio_wl.c b/components/fatfs/diskio/diskio_wl.c index 2a17f5ba0..5b1a5e6dd 100644 --- a/components/fatfs/diskio/diskio_wl.c +++ b/components/fatfs/diskio/diskio_wl.c @@ -43,7 +43,7 @@ DRESULT ff_wl_read (BYTE pdrv, BYTE *buff, DWORD sector, UINT count) wl_handle_t wl_handle = ff_wl_handles[pdrv]; assert(wl_handle + 1); esp_err_t err = wl_read(wl_handle, sector * wl_sector_size(wl_handle), buff, count * wl_sector_size(wl_handle)); - if (err != ESP_OK) { + if (unlikely(err != ESP_OK)) { ESP_LOGE(TAG, "wl_read failed (%d)", err); return RES_ERROR; } @@ -56,12 +56,12 @@ DRESULT ff_wl_write (BYTE pdrv, const BYTE *buff, DWORD sector, UINT count) wl_handle_t wl_handle = ff_wl_handles[pdrv]; assert(wl_handle + 1); esp_err_t err = wl_erase_range(wl_handle, sector * wl_sector_size(wl_handle), count * wl_sector_size(wl_handle)); - if (err != ESP_OK) { + if (unlikely(err != ESP_OK)) { ESP_LOGE(TAG, "wl_erase_range failed (%d)", err); return RES_ERROR; } err = wl_write(wl_handle, sector * wl_sector_size(wl_handle), buff, count * wl_sector_size(wl_handle)); - if (err != ESP_OK) { + if (unlikely(err != ESP_OK)) { ESP_LOGE(TAG, "wl_write failed (%d)", err); return RES_ERROR; } diff --git a/components/fatfs/diskio/diskio_wl.h b/components/fatfs/diskio/diskio_wl.h index af3f14797..bb5f1a9d6 100644 --- a/components/fatfs/diskio/diskio_wl.h +++ b/components/fatfs/diskio/diskio_wl.h @@ -20,6 +20,7 @@ extern "C" { #endif #include "wear_levelling.h" +#include "esp_compiler.h" /** diff --git a/components/freertos/event_groups.c b/components/freertos/event_groups.c index 3a06e2374..7b724891e 100644 --- a/components/freertos/event_groups.c +++ b/components/freertos/event_groups.c @@ -444,7 +444,7 @@ BaseType_t xTimeoutOccurred = pdFALSE; event list item, and they should now be retrieved then cleared. */ uxReturn = uxTaskResetEventItemValue(); - if( ( uxReturn & eventUNBLOCKED_DUE_TO_BIT_SET ) == ( EventBits_t ) 0 ) + if( unlikely (( uxReturn & eventUNBLOCKED_DUE_TO_BIT_SET ) == ( EventBits_t ) 0) ) { taskENTER_CRITICAL( &pxEventBits->eventGroupMux ); { diff --git a/components/freertos/include/freertos/FreeRTOS.h b/components/freertos/include/freertos/FreeRTOS.h index 207e65345..98f5dfac7 100644 --- a/components/freertos/include/freertos/FreeRTOS.h +++ b/components/freertos/include/freertos/FreeRTOS.h @@ -94,6 +94,8 @@ #ifdef __cplusplus extern "C" { #endif +/* for likely and unlikely */ +#include "esp_compiler.h" /* Application specific configuration options. */ #include "FreeRTOSConfig.h" diff --git a/components/freertos/include/freertos/FreeRTOSConfig.h b/components/freertos/include/freertos/FreeRTOSConfig.h index 139d14a40..af4bf29b1 100644 --- a/components/freertos/include/freertos/FreeRTOSConfig.h +++ b/components/freertos/include/freertos/FreeRTOSConfig.h @@ -126,12 +126,12 @@ int xt_clock_freq(void) __attribute__((deprecated)); #if defined(CONFIG_FREERTOS_ASSERT_DISABLE) #define configASSERT(a) /* assertions disabled */ #elif defined(CONFIG_FREERTOS_ASSERT_FAIL_PRINT_CONTINUE) -#define configASSERT(a) if (!(a)) { \ +#define configASSERT(a) if (unlikely(!(a))) { \ ets_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ __FUNCTION__); \ } #else /* CONFIG_FREERTOS_ASSERT_FAIL_ABORT */ -#define configASSERT(a) if (!(a)) { \ +#define configASSERT(a) if (unlikely(!(a))) { \ ets_printf("%s:%d (%s)- assert failed!\n", __FILE__, __LINE__, \ __FUNCTION__); \ abort(); \ diff --git a/components/freertos/list.c b/components/freertos/list.c index 03649295c..a1aee58d9 100644 --- a/components/freertos/list.c +++ b/components/freertos/list.c @@ -72,6 +72,7 @@ #include "FreeRTOS.h" #include "list.h" + /*----------------------------------------------------------- * PUBLIC LIST API documented in list.h *----------------------------------------------------------*/ @@ -215,7 +216,7 @@ List_t * const pxList = ( List_t * ) pxItemToRemove->pvContainer; pxItemToRemove->pxPrevious->pxNext = pxItemToRemove->pxNext; /* Make sure the index is left pointing to a valid item. */ - if( pxList->pxIndex == pxItemToRemove ) + if(likely(pxList->pxIndex == pxItemToRemove)) { pxList->pxIndex = pxItemToRemove->pxPrevious; } diff --git a/components/freertos/port.c b/components/freertos/port.c index bab93aad5..74879e3d5 100644 --- a/components/freertos/port.c +++ b/components/freertos/port.c @@ -109,6 +109,7 @@ #include "esp_intr_alloc.h" #include "esp_log.h" #include "sdkconfig.h" +#include "esp_compiler.h" /* Defined in portasm.h */ extern void _frxt_tick_timer_init(void); diff --git a/components/freertos/queue.c b/components/freertos/queue.c index 95de32e59..e39e14980 100644 --- a/components/freertos/queue.c +++ b/components/freertos/queue.c @@ -782,7 +782,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; mtCOVERAGE_TEST_MARKER(); } } - else if( xYieldRequired != pdFALSE ) + else if(unlikely(xYieldRequired != pdFALSE)) { /* This path is a special case that will only get executed if the task was holding multiple mutexes @@ -815,7 +815,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; mtCOVERAGE_TEST_MARKER(); } } - else if( xYieldRequired != pdFALSE ) + else if(unlikely(xYieldRequired != pdFALSE)) { /* This path is a special case that will only get executed if the task was holding multiple mutexes and @@ -868,7 +868,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; taskENTER_CRITICAL(&pxQueue->mux); /* Update the timeout state to see if it has expired yet. */ - if( xTaskCheckForTimeOut( &xTimeOut, &xTicksToWait ) == pdFALSE ) + if(likely(xTaskCheckForTimeOut( &xTimeOut, &xTicksToWait ) == pdFALSE )) { if( prvIsQueueFull( pxQueue ) != pdFALSE ) { @@ -1573,7 +1573,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; taskENTER_CRITICAL(&pxQueue->mux); /* Update the timeout state to see if it has expired yet. */ - if( xTaskCheckForTimeOut( &xTimeOut, &xTicksToWait ) == pdFALSE ) + if(likely(xTaskCheckForTimeOut( &xTimeOut, &xTicksToWait ) == pdFALSE)) { if( prvIsQueueEmpty( pxQueue ) != pdFALSE ) { @@ -1888,7 +1888,7 @@ BaseType_t xReturn = pdFALSE; } #endif /* configUSE_MUTEXES */ } - else if( xPosition == queueSEND_TO_BACK ) + else if(likely(xPosition == queueSEND_TO_BACK)) { ( void ) memcpy( ( void * ) pxQueue->pcWriteTo, pvItemToQueue, ( size_t ) pxQueue->uxItemSize ); /*lint !e961 !e418 MISRA exception as the casts are only redundant for some ports, plus previous logic ensures a null pointer can only be passed to memcpy() if the copy size is 0. */ pxQueue->pcWriteTo += pxQueue->uxItemSize; diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index fe8c2523c..ec0a8c5f2 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -77,6 +77,7 @@ all the API functions to use the MPU wrappers. That should only be done when task.h is included from an application file. */ #define MPU_WRAPPERS_INCLUDED_FROM_API_FILE #include "esp_newlib.h" +#include "esp_compiler.h" /* FreeRTOS includes. */ #include "FreeRTOS.h" @@ -2466,7 +2467,7 @@ BaseType_t xSwitchRequired = pdFALSE; /* Only allow core 0 increase the tick count in the case of xPortSysTickHandler processing. */ /* And allow core 0 and core 1 to unwind uxPendedTicks during xTaskResumeAll. */ - if ( xPortInIsrContext() ) + if (likely(xPortInIsrContext())) { #if ( configUSE_TICK_HOOK == 1 ) vApplicationTickHook(); @@ -4187,7 +4188,7 @@ For ESP32 FreeRTOS, vTaskEnterCritical implements both portENTER_CRITICAL and po { BaseType_t oldInterruptLevel=0; BaseType_t schedulerRunning = xSchedulerRunning; - if( schedulerRunning != pdFALSE ) + if(likely(schedulerRunning != pdFALSE)) { //Interrupts may already be disabled (because we're doing this recursively) but we can't get the interrupt level after //vPortCPUAquireMutex, because it also may mess with interrupts. Get it here first, then later figure out if we're nesting @@ -4200,7 +4201,7 @@ For ESP32 FreeRTOS, vTaskEnterCritical implements both portENTER_CRITICAL and po vPortCPUAcquireMutexIntsDisabled( mux, portMUX_NO_TIMEOUT ); #endif - if( schedulerRunning != pdFALSE ) + if(likely(schedulerRunning != pdFALSE)) { TCB_t *tcb = pxCurrentTCB[xPortGetCoreID()]; BaseType_t newNesting = tcb->uxCriticalNesting + 1; @@ -4259,11 +4260,11 @@ For ESP32 FreeRTOS, vTaskExitCritical implements both portEXIT_CRITICAL and port #else vPortCPUReleaseMutexIntsDisabled( mux ); #endif - if( xSchedulerRunning != pdFALSE ) + if(likely(xSchedulerRunning != pdFALSE)) { TCB_t *tcb = pxCurrentTCB[xPortGetCoreID()]; BaseType_t nesting = tcb->uxCriticalNesting; - if( nesting > 0U ) + if(likely(nesting > 0U)) { nesting--; tcb->uxCriticalNesting = nesting; diff --git a/components/freertos/timers.c b/components/freertos/timers.c index 3a86b6509..125a5357f 100644 --- a/components/freertos/timers.c +++ b/components/freertos/timers.c @@ -410,7 +410,7 @@ DaemonTaskMessage_t xMessage; /* Send a message to the timer service task to perform a particular action on a particular timer definition. */ - if( xTimerQueue != NULL ) + if(unlikely( xTimerQueue != NULL )) { /* Send a command to the timer service task to start the xTimer timer. */ xMessage.xMessageID = xCommandID; @@ -725,7 +725,7 @@ TickType_t xTimeNow; /* Commands that are positive are timer commands rather than pended function calls. */ - if( xMessage.xMessageID >= ( BaseType_t ) 0 ) + if(likely( xMessage.xMessageID >= ( BaseType_t ) 0) ) { /* The messages uses the xTimerParameters member to work on a software timer. */ @@ -759,7 +759,7 @@ TickType_t xTimeNow; case tmrCOMMAND_RESET_FROM_ISR : case tmrCOMMAND_START_DONT_TRACE : /* Start or restart a timer. */ - if( prvInsertTimerInActiveList( pxTimer, xMessage.u.xTimerParameters.xMessageValue + pxTimer->xTimerPeriodInTicks, xTimeNow, xMessage.u.xTimerParameters.xMessageValue ) == pdTRUE ) + if(likely( prvInsertTimerInActiveList( pxTimer, xMessage.u.xTimerParameters.xMessageValue + pxTimer->xTimerPeriodInTicks, xTimeNow, xMessage.u.xTimerParameters.xMessageValue ) == pdTRUE )) { /* The timer expired before it was added to the active timer list. Process it now. */ diff --git a/components/lwip/port/esp32/freertos/sys_arch.c b/components/lwip/port/esp32/freertos/sys_arch.c index 85bca91c4..c9d5b361c 100644 --- a/components/lwip/port/esp32/freertos/sys_arch.c +++ b/components/lwip/port/esp32/freertos/sys_arch.c @@ -40,6 +40,7 @@ #include "arch/sys_arch.h" #include "lwip/stats.h" #include "esp_log.h" +#include "esp_compiler.h" static const char* TAG = "lwip_arch"; diff --git a/components/lwip/port/esp32/netif/ethernetif.c b/components/lwip/port/esp32/netif/ethernetif.c index 3c75bd1eb..d513e181b 100644 --- a/components/lwip/port/esp32/netif/ethernetif.c +++ b/components/lwip/port/esp32/netif/ethernetif.c @@ -51,6 +51,7 @@ #include "esp_eth.h" #include "esp_netif.h" #include "esp_netif_net_stack.h" +#include "esp_compiler.h" /* Define those to better describe your network interface. */ #define IFNAME0 'e' @@ -133,7 +134,7 @@ static err_t ethernet_low_level_output(struct netif *netif, struct pbuf *p) pbuf_free(q); } /* Check error */ - if (ret != ESP_OK) { + if (unlikely(ret != ESP_OK)) { return ERR_ABRT; } else { return ERR_OK; @@ -156,7 +157,7 @@ void ethernetif_input(void *h, void *buffer, size_t len, void *eb) struct netif *netif = h; struct pbuf *p; - if (buffer == NULL || !netif_is_up(netif)) { + if (unlikely(buffer == NULL || !netif_is_up(netif))) { if (buffer) { ethernet_free_rx_buf_l2(netif, buffer); } @@ -175,7 +176,7 @@ void ethernetif_input(void *h, void *buffer, size_t len, void *eb) p->l2_buf = buffer; #endif /* full packet send to tcpip_thread to process */ - if (netif->input(p, netif) != ERR_OK) { + if (unlikely(netif->input(p, netif) != ERR_OK)) { LWIP_DEBUGF(NETIF_DEBUG, ("ethernetif_input: IP input error\n")); pbuf_free(p); } diff --git a/components/lwip/port/esp32/netif/wlanif.c b/components/lwip/port/esp32/netif/wlanif.c index e1d0bed6b..88f137fc3 100644 --- a/components/lwip/port/esp32/netif/wlanif.c +++ b/components/lwip/port/esp32/netif/wlanif.c @@ -52,6 +52,7 @@ #include "esp_netif.h" #include "esp_netif_net_stack.h" +#include "esp_compiler.h" /** * @brief Free resources allocated in L2 layer @@ -160,7 +161,7 @@ wlanif_input(void *h, void *buffer, size_t len, void* eb) esp_netif_t *esp_netif = esp_netif_get_handle_from_netif_impl(netif); struct pbuf *p; - if(!buffer || !netif_is_up(netif)) { + if(unlikely(!buffer || !netif_is_up(netif))) { if (eb) { esp_netif_free_rx_buffer(esp_netif, eb); } @@ -190,7 +191,7 @@ wlanif_input(void *h, void *buffer, size_t len, void* eb) #endif /* full packet send to tcpip_thread to process */ - if (netif->input(p, netif) != ERR_OK) { + if (unlikely(netif->input(p, netif) != ERR_OK)) { LWIP_DEBUGF(NETIF_DEBUG, ("ethernetif_input: IP input error\n")); pbuf_free(p); } diff --git a/components/sdmmc/sdmmc_io.c b/components/sdmmc/sdmmc_io.c index e77623a17..b7611f853 100644 --- a/components/sdmmc/sdmmc_io.c +++ b/components/sdmmc/sdmmc_io.c @@ -17,6 +17,7 @@ #include "sdmmc_common.h" #include "esp_attr.h" +#include "esp_compiler.h" #define CIS_TUPLE(NAME) (cis_tuple_t) {.code=CISTPL_CODE_##NAME, .name=#NAME, .func=&cis_tuple_func_default, } @@ -235,7 +236,7 @@ esp_err_t sdmmc_io_read_byte(sdmmc_card_t* card, uint32_t function, uint32_t addr, uint8_t *out_byte) { esp_err_t ret = sdmmc_io_rw_direct(card, function, addr, SD_ARG_CMD52_READ, out_byte); - if (ret != ESP_OK) { + if (unlikely(ret != ESP_OK)) { ESP_LOGE(TAG, "%s: sdmmc_io_rw_direct (read 0x%x) returned 0x%x", __func__, addr, ret); } return ret; @@ -247,7 +248,7 @@ esp_err_t sdmmc_io_write_byte(sdmmc_card_t* card, uint32_t function, uint8_t tmp_byte = in_byte; esp_err_t ret = sdmmc_io_rw_direct(card, function, addr, SD_ARG_CMD52_WRITE | SD_ARG_CMD52_EXCHANGE, &tmp_byte); - if (ret != ESP_OK) { + if (unlikely(ret != ESP_OK)) { ESP_LOGE(TAG, "%s: sdmmc_io_rw_direct (write 0x%x) returned 0x%x", __func__, addr, ret); return ret; } @@ -323,7 +324,7 @@ esp_err_t sdmmc_io_read_bytes(sdmmc_card_t* card, uint32_t function, esp_err_t err = sdmmc_io_rw_extended(card, function, addr, SD_ARG_CMD53_READ | SD_ARG_CMD53_INCREMENT, pc_dst, will_transfer); - if (err != ESP_OK) { + if (unlikely(err != ESP_OK)) { return err; } pc_dst += will_transfer; @@ -346,7 +347,7 @@ esp_err_t sdmmc_io_write_bytes(sdmmc_card_t* card, uint32_t function, esp_err_t err = sdmmc_io_rw_extended(card, function, addr, SD_ARG_CMD53_WRITE | SD_ARG_CMD53_INCREMENT, (void*) pc_src, will_transfer); - if (err != ESP_OK) { + if (unlikely(err != ESP_OK)) { return err; } pc_src += will_transfer; @@ -359,7 +360,7 @@ esp_err_t sdmmc_io_write_bytes(sdmmc_card_t* card, uint32_t function, esp_err_t sdmmc_io_read_blocks(sdmmc_card_t* card, uint32_t function, uint32_t addr, void* dst, size_t size) { - if (size % 4 != 0) { + if (unlikely(size % 4 != 0)) { return ESP_ERR_INVALID_SIZE; } return sdmmc_io_rw_extended(card, function, addr, @@ -370,7 +371,7 @@ esp_err_t sdmmc_io_read_blocks(sdmmc_card_t* card, uint32_t function, esp_err_t sdmmc_io_write_blocks(sdmmc_card_t* card, uint32_t function, uint32_t addr, const void* src, size_t size) { - if (size % 4 != 0) { + if (unlikely(size % 4 != 0)) { return ESP_ERR_INVALID_SIZE; } return sdmmc_io_rw_extended(card, function, addr, diff --git a/components/spiffs/spiffs_api.c b/components/spiffs/spiffs_api.c index 01c256d01..e628e8644 100644 --- a/components/spiffs/spiffs_api.c +++ b/components/spiffs/spiffs_api.c @@ -35,7 +35,7 @@ s32_t spiffs_api_read(spiffs *fs, uint32_t addr, uint32_t size, uint8_t *dst) { esp_err_t err = esp_partition_read(((esp_spiffs_t *)(fs->user_data))->partition, addr, dst, size); - if (err) { + if (unlikely(err)) { ESP_LOGE(TAG, "failed to read addr %08x, size %08x, err %d", addr, size, err); return -1; } @@ -46,7 +46,7 @@ s32_t spiffs_api_write(spiffs *fs, uint32_t addr, uint32_t size, uint8_t *src) { esp_err_t err = esp_partition_write(((esp_spiffs_t *)(fs->user_data))->partition, addr, src, size); - if (err) { + if (unlikely(err)) { ESP_LOGE(TAG, "failed to write addr %08x, size %08x, err %d", addr, size, err); return -1; } diff --git a/components/spiffs/spiffs_api.h b/components/spiffs/spiffs_api.h index 5009716a0..ced85da5a 100644 --- a/components/spiffs/spiffs_api.h +++ b/components/spiffs/spiffs_api.h @@ -21,6 +21,7 @@ #include "freertos/semphr.h" #include "spiffs.h" #include "esp_vfs.h" +#include "esp_compiler.h" #ifdef __cplusplus extern "C" { From 1b76253e0ef924268f056dc401c9cd1b64141d2f Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Fri, 25 Oct 2019 11:24:12 -0300 Subject: [PATCH 2/5] newlib/assert: placed unlikely macro as part of assertion newlib/assert: replace unlikely with likely to keep original assertion newlib/assert: fix assert macro that uses likely freertos/port: add the missing sdkconfig.h back newlib/assert: assert macro back to a single line --- components/freertos/port.c | 1 - components/newlib/platform_include/assert.h | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/components/freertos/port.c b/components/freertos/port.c index 74879e3d5..9bbff3236 100644 --- a/components/freertos/port.c +++ b/components/freertos/port.c @@ -90,7 +90,6 @@ // SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -------------------------------------------------------------------------------- */ - #include #include #include diff --git a/components/newlib/platform_include/assert.h b/components/newlib/platform_include/assert.h index 356872721..d800985b2 100644 --- a/components/newlib/platform_include/assert.h +++ b/components/newlib/platform_include/assert.h @@ -19,10 +19,12 @@ #pragma once #include #include +#include "esp_compiler.h" #include_next + #if defined(CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT) && !defined(NDEBUG) #undef assert -#define assert(__e) ((__e) ? (void)0 : abort()) +#define assert(__e) (likely(__e)) ? (void)0 : abort() #endif From 668b33dcf3ae3fac61984dc01dc3d42840ba897e Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Thu, 21 Nov 2019 12:17:57 -0300 Subject: [PATCH 3/5] esp_compiler: generate likely and unlikely macros only when performance optimization is selected as build option --- components/esp_common/include/esp_compiler.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/components/esp_common/include/esp_compiler.h b/components/esp_common/include/esp_compiler.h index 64ae87802..94ec29c23 100644 --- a/components/esp_common/include/esp_compiler.h +++ b/components/esp_common/include/esp_compiler.h @@ -22,7 +22,12 @@ * to reorder instructions producing more optimized * code. */ +#if (CONFIG_COMPILER_OPTIMIZATION_PERF) #define likely(x) __builtin_expect(!!(x), 1) #define unlikely(x) __builtin_expect(!!(x), 0) +#else +#define likely(x) (x) +#define unlikely(x) (x) +#endif #endif \ No newline at end of file From d059a955aecfc3897a003ac63137f3ac1463adef Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Wed, 4 Dec 2019 11:01:25 -0300 Subject: [PATCH 4/5] freertos: removed likely macros from non-port specifics parts of freertos fatfs: moved esp_compiler header file inside of disk implementation file --- components/fatfs/diskio/diskio_rawflash.c | 1 + components/fatfs/diskio/diskio_rawflash.h | 1 - components/fatfs/diskio/diskio_sdmmc.c | 1 + components/fatfs/diskio/diskio_sdmmc.h | 1 - components/fatfs/diskio/diskio_wl.c | 1 + components/fatfs/diskio/diskio_wl.h | 1 - components/freertos/event_groups.c | 2 +- components/freertos/list.c | 2 +- components/freertos/queue.c | 10 +++++----- components/freertos/tasks.c | 10 +++++----- components/freertos/timers.c | 6 +++--- 11 files changed, 18 insertions(+), 18 deletions(-) diff --git a/components/fatfs/diskio/diskio_rawflash.c b/components/fatfs/diskio/diskio_rawflash.c index 01fc05b90..382e53239 100644 --- a/components/fatfs/diskio/diskio_rawflash.c +++ b/components/fatfs/diskio/diskio_rawflash.c @@ -18,6 +18,7 @@ #include "ff.h" #include "esp_log.h" #include "diskio_rawflash.h" +#include "esp_compiler.h" static const char* TAG = "diskio_rawflash"; diff --git a/components/fatfs/diskio/diskio_rawflash.h b/components/fatfs/diskio/diskio_rawflash.h index 48d2dd76e..73ff15f88 100644 --- a/components/fatfs/diskio/diskio_rawflash.h +++ b/components/fatfs/diskio/diskio_rawflash.h @@ -20,7 +20,6 @@ extern "C" { #endif #include "esp_partition.h" -#include "esp_compiler.h" /** * Register spi flash partition diff --git a/components/fatfs/diskio/diskio_sdmmc.c b/components/fatfs/diskio/diskio_sdmmc.c index b1230f1a3..15e4a8677 100644 --- a/components/fatfs/diskio/diskio_sdmmc.c +++ b/components/fatfs/diskio/diskio_sdmmc.c @@ -17,6 +17,7 @@ #include "ff.h" #include "sdmmc_cmd.h" #include "esp_log.h" +#include "esp_compiler.h" static sdmmc_card_t* s_cards[FF_VOLUMES] = { NULL }; diff --git a/components/fatfs/diskio/diskio_sdmmc.h b/components/fatfs/diskio/diskio_sdmmc.h index 6036ab215..d7a50221b 100644 --- a/components/fatfs/diskio/diskio_sdmmc.h +++ b/components/fatfs/diskio/diskio_sdmmc.h @@ -16,7 +16,6 @@ #include "sdmmc_cmd.h" #include "driver/sdmmc_host.h" -#include "esp_compiler.h" #ifdef __cplusplus extern "C" { diff --git a/components/fatfs/diskio/diskio_wl.c b/components/fatfs/diskio/diskio_wl.c index 5b1a5e6dd..617236e8f 100644 --- a/components/fatfs/diskio/diskio_wl.c +++ b/components/fatfs/diskio/diskio_wl.c @@ -19,6 +19,7 @@ #include "esp_log.h" #include "diskio_wl.h" #include "wear_levelling.h" +#include "esp_compiler.h" static const char* TAG = "ff_diskio_spiflash"; diff --git a/components/fatfs/diskio/diskio_wl.h b/components/fatfs/diskio/diskio_wl.h index bb5f1a9d6..af3f14797 100644 --- a/components/fatfs/diskio/diskio_wl.h +++ b/components/fatfs/diskio/diskio_wl.h @@ -20,7 +20,6 @@ extern "C" { #endif #include "wear_levelling.h" -#include "esp_compiler.h" /** diff --git a/components/freertos/event_groups.c b/components/freertos/event_groups.c index 7b724891e..3a06e2374 100644 --- a/components/freertos/event_groups.c +++ b/components/freertos/event_groups.c @@ -444,7 +444,7 @@ BaseType_t xTimeoutOccurred = pdFALSE; event list item, and they should now be retrieved then cleared. */ uxReturn = uxTaskResetEventItemValue(); - if( unlikely (( uxReturn & eventUNBLOCKED_DUE_TO_BIT_SET ) == ( EventBits_t ) 0) ) + if( ( uxReturn & eventUNBLOCKED_DUE_TO_BIT_SET ) == ( EventBits_t ) 0 ) { taskENTER_CRITICAL( &pxEventBits->eventGroupMux ); { diff --git a/components/freertos/list.c b/components/freertos/list.c index a1aee58d9..708163388 100644 --- a/components/freertos/list.c +++ b/components/freertos/list.c @@ -216,7 +216,7 @@ List_t * const pxList = ( List_t * ) pxItemToRemove->pvContainer; pxItemToRemove->pxPrevious->pxNext = pxItemToRemove->pxNext; /* Make sure the index is left pointing to a valid item. */ - if(likely(pxList->pxIndex == pxItemToRemove)) + if(pxList->pxIndex == pxItemToRemove) { pxList->pxIndex = pxItemToRemove->pxPrevious; } diff --git a/components/freertos/queue.c b/components/freertos/queue.c index e39e14980..e0d8738a4 100644 --- a/components/freertos/queue.c +++ b/components/freertos/queue.c @@ -782,7 +782,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; mtCOVERAGE_TEST_MARKER(); } } - else if(unlikely(xYieldRequired != pdFALSE)) + else if(xYieldRequired != pdFALSE) { /* This path is a special case that will only get executed if the task was holding multiple mutexes @@ -815,7 +815,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; mtCOVERAGE_TEST_MARKER(); } } - else if(unlikely(xYieldRequired != pdFALSE)) + else if(xYieldRequired != pdFALSE) { /* This path is a special case that will only get executed if the task was holding multiple mutexes and @@ -868,7 +868,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; taskENTER_CRITICAL(&pxQueue->mux); /* Update the timeout state to see if it has expired yet. */ - if(likely(xTaskCheckForTimeOut( &xTimeOut, &xTicksToWait ) == pdFALSE )) + if(xTaskCheckForTimeOut( &xTimeOut, &xTicksToWait ) == pdFALSE ) { if( prvIsQueueFull( pxQueue ) != pdFALSE ) { @@ -1573,7 +1573,7 @@ Queue_t * const pxQueue = ( Queue_t * ) xQueue; taskENTER_CRITICAL(&pxQueue->mux); /* Update the timeout state to see if it has expired yet. */ - if(likely(xTaskCheckForTimeOut( &xTimeOut, &xTicksToWait ) == pdFALSE)) + if(xTaskCheckForTimeOut( &xTimeOut, &xTicksToWait ) == pdFALSE) { if( prvIsQueueEmpty( pxQueue ) != pdFALSE ) { @@ -1888,7 +1888,7 @@ BaseType_t xReturn = pdFALSE; } #endif /* configUSE_MUTEXES */ } - else if(likely(xPosition == queueSEND_TO_BACK)) + else if(xPosition == queueSEND_TO_BACK) { ( void ) memcpy( ( void * ) pxQueue->pcWriteTo, pvItemToQueue, ( size_t ) pxQueue->uxItemSize ); /*lint !e961 !e418 MISRA exception as the casts are only redundant for some ports, plus previous logic ensures a null pointer can only be passed to memcpy() if the copy size is 0. */ pxQueue->pcWriteTo += pxQueue->uxItemSize; diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index ec0a8c5f2..564b4aa4b 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -2467,7 +2467,7 @@ BaseType_t xSwitchRequired = pdFALSE; /* Only allow core 0 increase the tick count in the case of xPortSysTickHandler processing. */ /* And allow core 0 and core 1 to unwind uxPendedTicks during xTaskResumeAll. */ - if (likely(xPortInIsrContext())) + if (xPortInIsrContext()) { #if ( configUSE_TICK_HOOK == 1 ) vApplicationTickHook(); @@ -4188,7 +4188,7 @@ For ESP32 FreeRTOS, vTaskEnterCritical implements both portENTER_CRITICAL and po { BaseType_t oldInterruptLevel=0; BaseType_t schedulerRunning = xSchedulerRunning; - if(likely(schedulerRunning != pdFALSE)) + if(schedulerRunning != pdFALSE) { //Interrupts may already be disabled (because we're doing this recursively) but we can't get the interrupt level after //vPortCPUAquireMutex, because it also may mess with interrupts. Get it here first, then later figure out if we're nesting @@ -4201,7 +4201,7 @@ For ESP32 FreeRTOS, vTaskEnterCritical implements both portENTER_CRITICAL and po vPortCPUAcquireMutexIntsDisabled( mux, portMUX_NO_TIMEOUT ); #endif - if(likely(schedulerRunning != pdFALSE)) + if(schedulerRunning != pdFALSE) { TCB_t *tcb = pxCurrentTCB[xPortGetCoreID()]; BaseType_t newNesting = tcb->uxCriticalNesting + 1; @@ -4260,11 +4260,11 @@ For ESP32 FreeRTOS, vTaskExitCritical implements both portEXIT_CRITICAL and port #else vPortCPUReleaseMutexIntsDisabled( mux ); #endif - if(likely(xSchedulerRunning != pdFALSE)) + if(xSchedulerRunning != pdFALSE) { TCB_t *tcb = pxCurrentTCB[xPortGetCoreID()]; BaseType_t nesting = tcb->uxCriticalNesting; - if(likely(nesting > 0U)) + if(nesting > 0U) { nesting--; tcb->uxCriticalNesting = nesting; diff --git a/components/freertos/timers.c b/components/freertos/timers.c index 125a5357f..3a86b6509 100644 --- a/components/freertos/timers.c +++ b/components/freertos/timers.c @@ -410,7 +410,7 @@ DaemonTaskMessage_t xMessage; /* Send a message to the timer service task to perform a particular action on a particular timer definition. */ - if(unlikely( xTimerQueue != NULL )) + if( xTimerQueue != NULL ) { /* Send a command to the timer service task to start the xTimer timer. */ xMessage.xMessageID = xCommandID; @@ -725,7 +725,7 @@ TickType_t xTimeNow; /* Commands that are positive are timer commands rather than pended function calls. */ - if(likely( xMessage.xMessageID >= ( BaseType_t ) 0) ) + if( xMessage.xMessageID >= ( BaseType_t ) 0 ) { /* The messages uses the xTimerParameters member to work on a software timer. */ @@ -759,7 +759,7 @@ TickType_t xTimeNow; case tmrCOMMAND_RESET_FROM_ISR : case tmrCOMMAND_START_DONT_TRACE : /* Start or restart a timer. */ - if(likely( prvInsertTimerInActiveList( pxTimer, xMessage.u.xTimerParameters.xMessageValue + pxTimer->xTimerPeriodInTicks, xTimeNow, xMessage.u.xTimerParameters.xMessageValue ) == pdTRUE )) + if( prvInsertTimerInActiveList( pxTimer, xMessage.u.xTimerParameters.xMessageValue + pxTimer->xTimerPeriodInTicks, xTimeNow, xMessage.u.xTimerParameters.xMessageValue ) == pdTRUE ) { /* The timer expired before it was added to the active timer list. Process it now. */ From 5db46ab9e64564705443eba0f7e00c7fd71c2366 Mon Sep 17 00:00:00 2001 From: Felipe Neves Date: Fri, 3 Jan 2020 14:39:25 -0300 Subject: [PATCH 5/5] assert: extend likely macro to be called when silent assertions are off --- components/newlib/platform_include/assert.h | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/components/newlib/platform_include/assert.h b/components/newlib/platform_include/assert.h index d800985b2..ba01c798c 100644 --- a/components/newlib/platform_include/assert.h +++ b/components/newlib/platform_include/assert.h @@ -23,8 +23,19 @@ #include_next - #if defined(CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT) && !defined(NDEBUG) -#undef assert -#define assert(__e) (likely(__e)) ? (void)0 : abort() + #undef assert + #define assert(__e) (likely(__e)) ? (void)0 : abort() +#else + /* moved part of toolchain provided assert to there then + * we can tweak the original assert macro to perform likely + * before deliver it to original toolchain implementation + */ + #undef assert + #ifdef NDEBUG + # define assert(__e) ((void)0) + #else + # define assert(__e) (likely(__e) ? (void)0 : __assert_func (__FILE__, __LINE__, \ + __ASSERT_FUNC, #__e)) + #endif #endif