From 182184567e1939a56aeef325e0846cc3b364d9f8 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 17 Oct 2016 12:38:17 +0800 Subject: [PATCH 1/5] build system: add menuconfig choice for optimization level, reorganize C*FLAGS This change adds two options (Debug/Release) for optimization level. Debug enables -O0, release enables -Os and adds -DNDEBUG (which removes all assert() statements). Debugging symbols are kept in both cases, although we may add an option to strip output file if necessary. Also we used to define all common compiler flags in CPPFLAGS, and then appended them to CFLAGS/CXXFLAGS. It makes it impossible to add preprocessor macros to CPPFLAGS at component level (one has to use CFLAGS/CXXFLAGS instead). Some third party libraries are not compatible with this approach. Changed to the more common way of using these variables. --- Kconfig | 11 +++++++++++ components/log/log.c | 2 +- make/component_common.mk | 10 +++++----- make/project.mk | 35 ++++++++++++++++++++++++++++------- 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/Kconfig b/Kconfig index 11ea099de..936181a9c 100644 --- a/Kconfig +++ b/Kconfig @@ -23,6 +23,17 @@ endmenu source "$COMPONENT_KCONFIGS_PROJBUILD" +choice OPTIMIZATION_LEVEL + prompt "Optimization level" + default OPTIMIZATION_LEVEL_DEBUG + help + This option sets compiler optimization level. +config OPTIMIZATION_LEVEL_DEBUG + bool "Debug" +config OPTIMIZATION_LEVEL_RELEASE + bool "Release" +endchoice + menu "Component config" source "$COMPONENT_KCONFIGS" endmenu diff --git a/components/log/log.c b/components/log/log.c index aae12a773..a2b41d7e6 100644 --- a/components/log/log.c +++ b/components/log/log.c @@ -284,7 +284,7 @@ static inline void heap_swap(int i, int j) } #endif //BOOTLOADER_BUILD -inline IRAM_ATTR uint32_t esp_log_early_timestamp() +IRAM_ATTR uint32_t esp_log_early_timestamp() { return xthal_get_ccount() / (CPU_CLK_FREQ_ROM / 1000); } diff --git a/make/component_common.mk b/make/component_common.mk index ebad525a7..bf2eace01 100644 --- a/make/component_common.mk +++ b/make/component_common.mk @@ -91,15 +91,15 @@ define GenerateCompileTargets # $(1) - directory containing source files, relative to $(COMPONENT_PATH) $(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.c | $(1) $$(summary) CC $$@ - $$(Q) $$(CC) $$(CFLAGS) $$(addprefix -I ,$$(COMPONENT_INCLUDES)) $$(addprefix -I ,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@ + $$(Q) $$(CC) $$(CFLAGS) $(CPPFLAGS) $$(addprefix -I ,$$(COMPONENT_INCLUDES)) $$(addprefix -I ,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@ $(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.cpp | $(1) - $$(summary) CC $$@ - $$(Q) $$(CXX) $$(CXXFLAGS) $$(addprefix -I,$$(COMPONENT_INCLUDES)) $$(addprefix -I,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@ + $$(summary) CXX $$@ + $$(Q) $$(CXX) $$(CXXFLAGS) $(CPPFLAGS) $$(addprefix -I,$$(COMPONENT_INCLUDES)) $$(addprefix -I,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@ $(1)/%.o: $$(COMPONENT_PATH)/$(1)/%.S | $(1) - $$(summary) CC $$@ - $$(Q) $$(CC) $$(CFLAGS) $$(addprefix -I ,$$(COMPONENT_INCLUDES)) $$(addprefix -I ,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@ + $$(summary) AS $$@ + $$(Q) $$(CC) $$(CFLAGS) $(CPPFLAGS) $$(addprefix -I ,$$(COMPONENT_INCLUDES)) $$(addprefix -I ,$$(COMPONENT_EXTRA_INCLUDES)) -I$(1) -c $$< -o $$@ # CWD is build dir, create the build subdirectory if it doesn't exist $(1): diff --git a/make/project.mk b/make/project.mk index d91165cfd..49c349946 100644 --- a/make/project.mk +++ b/make/project.mk @@ -157,15 +157,36 @@ LDFLAGS ?= -nostdlib \ # If you need your component to add CFLAGS/etc globally for all source # files, set CFLAGS += in your component's Makefile.projbuild -# CPPFLAGS used by an compile pass that uses the C preprocessor -CPPFLAGS = -DESP_PLATFORM -Og -g3 -Wpointer-arith -Werror -Wno-error=unused-function -Wno-error=unused-but-set-variable \ - -Wno-error=unused-variable -Wall -ffunction-sections -fdata-sections -mlongcalls -nostdlib -MMD -MP +# CPPFLAGS used by C preprocessor +CPPFLAGS = -DESP_PLATFORM -# C flags use by C only -CFLAGS = $(CPPFLAGS) -std=gnu99 -g3 -fstrict-volatile-bitfields +# Warnings-related flags relevant both for C and C++ +COMMON_WARNING_FLAGS = -Wall -Werror \ + -Wno-error=unused-function \ + -Wno-error=unused-but-set-variable \ + -Wno-error=unused-variable -# CXXFLAGS uses by C++ only -CXXFLAGS = $(CPPFLAGS) -Og -std=gnu++11 -g3 -fno-exceptions -fstrict-volatile-bitfields -fno-rtti +# Flags which control code generation and dependency generation, both for C and C++ +COMMON_FLAGS = \ + -ffunction-sections -fdata-sections \ + -fstrict-volatile-bitfields \ + -mlongcalls \ + -nostdlib \ + -MMD -MP + +# Optimization flags are set based on menuconfig choice +ifneq ("$(CONFIG_OPTIMIZATION_LEVEL_RELEASE)","") +OPTMIZATION_FLAGS = -Os +CPPFLAGS += -DNDEBUG +else +OPTMIZATION_FLAGS = -O0 +endif + +# List of flags to pass to C compiler +CFLAGS = -ggdb -std=gnu99 $(strip $(OPTMIZATION_FLAGS) $(COMMON_FLAGS) $(COMMON_WARNING_FLAGS)) + +# List of flags to pass to C++ compiler +CXXFLAGS = -ggdb -std=gnu++11 -fno-exceptions -fno-rtti $(strip $(OPTMIZATION_FLAGS) $(COMMON_FLAGS) $(COMMON_WARNING_FLAGS)) export CFLAGS CPPFLAGS CXXFLAGS From 1cd572c7b982cd876d2f16958c1b2d75cf5f0ea5 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 17 Oct 2016 12:45:42 +0800 Subject: [PATCH 2/5] Add test for compiling in release mode, fix warnings and errors which appeared --- .gitlab-ci.yml | 6 ++++++ components/nghttp/library/nghttp2_session.c | 2 +- components/nvs_flash/src/nvs_page.cpp | 2 +- components/nvs_flash/src/nvs_pagemanager.cpp | 4 +++- components/nvs_flash/src/nvs_storage.cpp | 3 +-- 5 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index dd4049358..aff9bea8d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -37,6 +37,12 @@ build_template_app: # branch - git checkout ${CI_BUILD_REF_NAME} || echo "Using esp-idf-template default branch..." - make defconfig + # Test debug build (default) + - make all V=1 + # Now test release build + - make clean + - sed -i.bak -e's/CONFIG_OPTIMIZATION_LEVEL_DEBUG\=y/CONFIG_OPTIMIZATION_LEVEL_RELEASE=y/' sdkconfig + - make defconfig - make all V=1 diff --git a/components/nghttp/library/nghttp2_session.c b/components/nghttp/library/nghttp2_session.c index f93cd1729..0100dd32c 100644 --- a/components/nghttp/library/nghttp2_session.c +++ b/components/nghttp/library/nghttp2_session.c @@ -7177,7 +7177,7 @@ uint32_t nghttp2_session_get_remote_settings(nghttp2_session *session, return session->remote_settings.max_header_list_size; } - assert(0); + abort(); } static int nghttp2_session_upgrade_internal(nghttp2_session *session, diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index 4f6a198c6..f4fc5430c 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -769,7 +769,7 @@ void Page::invalidateCache() void Page::debugDump() const { - printf("state=%x addr=%x seq=%d\nfirstUsed=%d nextFree=%d used=%d erased=%d\n", mState, mBaseAddress, mSeqNumber, static_cast(mFirstUsedEntry), static_cast(mNextFreeEntry), mUsedEntryCount, mErasedEntryCount); + printf("state=%x addr=%x seq=%d\nfirstUsed=%d nextFree=%d used=%d erased=%d\n", (int) mState, mBaseAddress, mSeqNumber, static_cast(mFirstUsedEntry), static_cast(mNextFreeEntry), mUsedEntryCount, mErasedEntryCount); size_t skip = 0; for (size_t i = 0; i < ENTRY_COUNT; ++i) { printf("%3d: ", static_cast(i)); diff --git a/components/nvs_flash/src/nvs_pagemanager.cpp b/components/nvs_flash/src/nvs_pagemanager.cpp index 790ab7e19..f4d02a7d4 100644 --- a/components/nvs_flash/src/nvs_pagemanager.cpp +++ b/components/nvs_flash/src/nvs_pagemanager.cpp @@ -49,7 +49,7 @@ esp_err_t PageManager::load(uint32_t baseSector, uint32_t sectorCount) return activatePage(); } else { uint32_t lastSeqNo; - assert(mPageList.back().getSeqNumber(lastSeqNo) == ESP_OK); + ESP_ERROR_CHECK( mPageList.back().getSeqNumber(lastSeqNo) ); mSeqNumber = lastSeqNo + 1; } @@ -142,7 +142,9 @@ esp_err_t PageManager::requestNewPage() Page* newPage = &mPageList.back(); Page* erasedPage = maxErasedItemsPageIt; +#ifndef NDEBUG size_t usedEntries = erasedPage->getUsedEntryCount(); +#endif err = erasedPage->markFreeing(); if (err != ESP_OK) { return err; diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index 4a217ebe1..eb90cac5b 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -123,8 +123,7 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key if (findPage) { if (findPage->state() == Page::PageState::UNINITIALIZED || findPage->state() == Page::PageState::INVALID) { - auto err = findItem(nsIndex, datatype, key, findPage, item); - assert(err == ESP_OK); + ESP_ERROR_CHECK( findItem(nsIndex, datatype, key, findPage, item) ); } err = findPage->eraseItem(nsIndex, datatype, key); if (err == ESP_ERR_FLASH_OP_FAIL) { From 34fa6a60a9812b4a91ac16535ad5ee86d5755a22 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 17 Oct 2016 13:47:13 +0800 Subject: [PATCH 3/5] build system: fix typo, move -ggdb to OPTIMIZATION_FLAGS --- make/project.mk | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/make/project.mk b/make/project.mk index 49c349946..9088eda85 100644 --- a/make/project.mk +++ b/make/project.mk @@ -176,17 +176,20 @@ COMMON_FLAGS = \ # Optimization flags are set based on menuconfig choice ifneq ("$(CONFIG_OPTIMIZATION_LEVEL_RELEASE)","") -OPTMIZATION_FLAGS = -Os +OPTIMIZATION_FLAGS = -Os CPPFLAGS += -DNDEBUG else -OPTMIZATION_FLAGS = -O0 +OPTIMIZATION_FLAGS = -O0 endif +# Enable generation of debugging symbols +OPTIMIZATION_FLAGS += -ggdb + # List of flags to pass to C compiler -CFLAGS = -ggdb -std=gnu99 $(strip $(OPTMIZATION_FLAGS) $(COMMON_FLAGS) $(COMMON_WARNING_FLAGS)) +CFLAGS = -std=gnu99 $(strip $(OPTIMIZATION_FLAGS) $(COMMON_FLAGS) $(COMMON_WARNING_FLAGS)) # List of flags to pass to C++ compiler -CXXFLAGS = -ggdb -std=gnu++11 -fno-exceptions -fno-rtti $(strip $(OPTMIZATION_FLAGS) $(COMMON_FLAGS) $(COMMON_WARNING_FLAGS)) +CXXFLAGS = -std=gnu++11 -fno-exceptions -fno-rtti $(strip $(OPTIMIZATION_FLAGS) $(COMMON_FLAGS) $(COMMON_WARNING_FLAGS)) export CFLAGS CPPFLAGS CXXFLAGS From 39a06319e245b7b4bd9ec5f8dbfe12d00a1c7940 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 20 Oct 2016 16:10:51 +0800 Subject: [PATCH 4/5] build system: use -Og instead of -O0 for debug builds, expand help text in menuconfig --- Kconfig | 5 ++++- make/project.mk | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Kconfig b/Kconfig index 936181a9c..97da1f01f 100644 --- a/Kconfig +++ b/Kconfig @@ -27,7 +27,10 @@ choice OPTIMIZATION_LEVEL prompt "Optimization level" default OPTIMIZATION_LEVEL_DEBUG help - This option sets compiler optimization level. + This option sets optimization level. + For "Release" setting, -Os flag is added to CFLAGS, + and -DNDEBUG flag is added to CPPFLAGS. + For "Debug" setting, -Og flag is added to CFLAGS. config OPTIMIZATION_LEVEL_DEBUG bool "Debug" config OPTIMIZATION_LEVEL_RELEASE diff --git a/make/project.mk b/make/project.mk index 9088eda85..b646dfc41 100644 --- a/make/project.mk +++ b/make/project.mk @@ -179,7 +179,7 @@ ifneq ("$(CONFIG_OPTIMIZATION_LEVEL_RELEASE)","") OPTIMIZATION_FLAGS = -Os CPPFLAGS += -DNDEBUG else -OPTIMIZATION_FLAGS = -O0 +OPTIMIZATION_FLAGS = -Og endif # Enable generation of debugging symbols From dfe0dcaed477023443bda6fa2319132b1f5bf20f Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 20 Oct 2016 17:17:54 +0800 Subject: [PATCH 5/5] build system: fix setting C**FLAGS from project makefile --- Kconfig | 10 ++++++++-- make/project.mk | 28 +++++++++++++++++++++------- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/Kconfig b/Kconfig index 97da1f01f..deb0cea83 100644 --- a/Kconfig +++ b/Kconfig @@ -28,9 +28,15 @@ choice OPTIMIZATION_LEVEL default OPTIMIZATION_LEVEL_DEBUG help This option sets optimization level. - For "Release" setting, -Os flag is added to CFLAGS, + + - for "Release" setting, -Os flag is added to CFLAGS, and -DNDEBUG flag is added to CPPFLAGS. - For "Debug" setting, -Og flag is added to CFLAGS. + + - for "Debug" setting, -Og flag is added to CFLAGS. + + To override any of these settings, set CFLAGS and/or CPPFLAGS + in project makefile, before including $(IDF_PATH)/make/project.mk. + config OPTIMIZATION_LEVEL_DEBUG bool "Debug" config OPTIMIZATION_LEVEL_RELEASE diff --git a/make/project.mk b/make/project.mk index b646dfc41..887086fd4 100644 --- a/make/project.mk +++ b/make/project.mk @@ -149,16 +149,16 @@ LDFLAGS ?= -nostdlib \ -Wl,-EL # Set default CPPFLAGS, CFLAGS, CXXFLAGS -# # These are exported so that components can use them when compiling. -# # If you need your component to add CFLAGS/etc for it's own source compilation only, set CFLAGS += in your component's Makefile. -# # If you need your component to add CFLAGS/etc globally for all source -# files, set CFLAGS += in your component's Makefile.projbuild +# files, set CFLAGS += in your component's Makefile.projbuild +# If you need to set CFLAGS/CPPFLAGS/CXXFLAGS at project level, set them in application Makefile +# before including project.mk. Default flags will be added before the ones provided in application Makefile. # CPPFLAGS used by C preprocessor -CPPFLAGS = -DESP_PLATFORM +# If any flags are defined in application Makefile, add them at the end. +CPPFLAGS := -DESP_PLATFORM $(CPPFLAGS) # Warnings-related flags relevant both for C and C++ COMMON_WARNING_FLAGS = -Wall -Werror \ @@ -186,10 +186,24 @@ endif OPTIMIZATION_FLAGS += -ggdb # List of flags to pass to C compiler -CFLAGS = -std=gnu99 $(strip $(OPTIMIZATION_FLAGS) $(COMMON_FLAGS) $(COMMON_WARNING_FLAGS)) +# If any flags are defined in application Makefile, add them at the end. +CFLAGS := $(strip \ + -std=gnu99 \ + $(OPTIMIZATION_FLAGS) \ + $(COMMON_FLAGS) \ + $(COMMON_WARNING_FLAGS) \ + $(CFLAGS)) # List of flags to pass to C++ compiler -CXXFLAGS = -std=gnu++11 -fno-exceptions -fno-rtti $(strip $(OPTIMIZATION_FLAGS) $(COMMON_FLAGS) $(COMMON_WARNING_FLAGS)) +# If any flags are defined in application Makefile, add them at the end. +CXXFLAGS := $(strip \ + -std=gnu++11 \ + -fno-exceptions \ + -fno-rtti \ + $(OPTIMIZATION_FLAGS) \ + $(COMMON_FLAGS) \ + $(COMMON_WARNING_FLAGS) \ + $(CXXFLAGS)) export CFLAGS CPPFLAGS CXXFLAGS