From f7a41fb493a8ae74cea698d45f66470a9b30ad10 Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Thu, 10 Oct 2024 13:37:00 +0100 Subject: [PATCH 1/2] perf(build): be clever about uppercasing Most of the macros in build_macros.mk get lazily evaluated. That's mostly fine, except for the fact that the `uppercase` macro needs to spawn a subshell to get its output. And the target for every file requires calling `uppercase` many, MANY, times, thrashing performance on even the most trivial of make commands. We can be a little clever and only call `uppercase` a handful of times and then pass around the already uppercased strings. The same is true about the verbosity augmentation variables. Simply changing them to simply expanded variables allows for them to be pre-processed and then used over and over again. `make realclean` is a pretty good benchmark for this as it doesn't do much else but must process all the rules, like every other make command. On a clean checkout of TF-A on an Intel Xeon Gold 5218 (i.e. slow single-core) workstation, that command used to take about 7 seconds. With this patch it takes about 0.5. Change-Id: I632236a12a40f169e834974ecbc73ff80aac3462 Signed-off-by: Boyan Karatotev --- make_helpers/build_macros.mk | 72 ++++++++++++++++------------- make_helpers/common.mk | 8 ++-- plat/arm/board/arm_fpga/platform.mk | 6 +-- 3 files changed, 48 insertions(+), 38 deletions(-) diff --git a/make_helpers/build_macros.mk b/make_helpers/build_macros.mk index f523074c4..b6e64219f 100644 --- a/make_helpers/build_macros.mk +++ b/make_helpers/build_macros.mk @@ -183,7 +183,7 @@ endef define TOOL_ADD_IMG_PAYLOAD -$(eval PRE_TOOL_FILTER := $($(call uppercase,$(1))_PRE_TOOL_FILTER)) +$(eval PRE_TOOL_FILTER := $($(1)_PRE_TOOL_FILTER)) ifneq ($(PRE_TOOL_FILTER),) @@ -220,7 +220,8 @@ endef define TOOL_ADD_IMG # Build option to specify the image filename (SCP_BL2, BL33, etc) # This is the uppercase form of the first parameter - $(eval _V := $(call uppercase,$(1))) + $(eval BL := $(call uppercase,$(1))) + $(eval _V := $(BL)) # $(check_$(1)_cmd) variable is executed in the check_$(1) target and also # is put into the ${CHECK_$(3)FIP_CMD} variable which is executed by the @@ -235,10 +236,10 @@ define TOOL_ADD_IMG ifeq ($(4),1) $(eval ENC_BIN := ${BUILD_PLAT}/$(1)_enc.bin) $(call ENCRYPT_FW,$(value $(_V)),$(ENC_BIN)) - $(call TOOL_ADD_IMG_PAYLOAD,$(1),$(value $(_V)),$(2),$(ENC_BIN),$(3), \ + $(call TOOL_ADD_IMG_PAYLOAD,$(BL),$(value $(_V)),$(2),$(ENC_BIN),$(3), \ $(ENC_BIN)) else - $(call TOOL_ADD_IMG_PAYLOAD,$(1),$(value $(_V)),$(2),$(if $(wildcard $(value $(_V))),$(value $(_V)),FORCE),$(3)) + $(call TOOL_ADD_IMG_PAYLOAD,$(BL),$(value $(_V)),$(2),$(if $(wildcard $(value $(_V))),$(value $(_V)),FORCE),$(3)) endif .PHONY: check_$(1) @@ -284,10 +285,11 @@ MAKE_DEP = -Wp,-MD,$(DEP) -MT $$@ -MP # $(1) = output directory # $(2) = source file (%.c) # $(3) = library name +# $(4) = uppercase name of the library define MAKE_C_LIB $(eval OBJ := $(1)/$(patsubst %.c,%.o,$(notdir $(2)))) $(eval DEP := $(patsubst %.o,%.d,$(OBJ))) -$(eval LIB := $(call uppercase, $(notdir $(1)))) +$(eval LIB := $(notdir $(1))) $(OBJ): $(2) $(filter-out %.d,$(MAKEFILE_LIST)) | $$$$(@D)/ $$(s)echo " CC $$<" @@ -301,6 +303,7 @@ endef # $(1) = output directory # $(2) = source file (%.S) # $(3) = library name +# $(4) = uppercase name of the library define MAKE_S_LIB $(eval OBJ := $(1)/$(patsubst %.S,%.o,$(notdir $(2)))) $(eval DEP := $(patsubst %.o,%.d,$(OBJ))) @@ -318,15 +321,16 @@ endef # $(1) = output directory # $(2) = source file (%.c) # $(3) = BL stage +# $(4) = uppercase BL stage define MAKE_C $(eval OBJ := $(1)/$(patsubst %.c,%.o,$(notdir $(2)))) $(eval DEP := $(patsubst %.o,%.d,$(OBJ))) -$(eval BL_DEFINES := IMAGE_$(call uppercase,$(3)) $($(call uppercase,$(3))_DEFINES) $(PLAT_BL_COMMON_DEFINES)) -$(eval BL_INCLUDE_DIRS := $($(call uppercase,$(3))_INCLUDE_DIRS) $(PLAT_BL_COMMON_INCLUDE_DIRS)) -$(eval BL_CPPFLAGS := $($(call uppercase,$(3))_CPPFLAGS) $(addprefix -D,$(BL_DEFINES)) $(addprefix -I,$(BL_INCLUDE_DIRS)) $(PLAT_BL_COMMON_CPPFLAGS)) -$(eval BL_CFLAGS := $($(call uppercase,$(3))_CFLAGS) $(PLAT_BL_COMMON_CFLAGS)) +$(eval BL_DEFINES := IMAGE_$(4) $($(4)_DEFINES) $(PLAT_BL_COMMON_DEFINES)) +$(eval BL_INCLUDE_DIRS := $($(4)_INCLUDE_DIRS) $(PLAT_BL_COMMON_INCLUDE_DIRS)) +$(eval BL_CPPFLAGS := $($(4)_CPPFLAGS) $(addprefix -D,$(BL_DEFINES)) $(addprefix -I,$(BL_INCLUDE_DIRS)) $(PLAT_BL_COMMON_CPPFLAGS)) +$(eval BL_CFLAGS := $($(4)_CFLAGS) $(PLAT_BL_COMMON_CFLAGS)) $(OBJ): $(2) $(filter-out %.d,$(MAKEFILE_LIST)) | $$$$(@D)/ $$(s)echo " CC $$<" @@ -341,15 +345,16 @@ endef # $(1) = output directory # $(2) = assembly file (%.S) # $(3) = BL stage +# $(4) = uppercase BL stage define MAKE_S $(eval OBJ := $(1)/$(patsubst %.S,%.o,$(notdir $(2)))) $(eval DEP := $(patsubst %.o,%.d,$(OBJ))) -$(eval BL_DEFINES := IMAGE_$(call uppercase,$(3)) $($(call uppercase,$(3))_DEFINES) $(PLAT_BL_COMMON_DEFINES)) -$(eval BL_INCLUDE_DIRS := $($(call uppercase,$(3))_INCLUDE_DIRS) $(PLAT_BL_COMMON_INCLUDE_DIRS)) -$(eval BL_CPPFLAGS := $($(call uppercase,$(3))_CPPFLAGS) $(addprefix -D,$(BL_DEFINES)) $(addprefix -I,$(BL_INCLUDE_DIRS)) $(PLAT_BL_COMMON_CPPFLAGS)) -$(eval BL_ASFLAGS := $($(call uppercase,$(3))_ASFLAGS) $(PLAT_BL_COMMON_ASFLAGS)) +$(eval BL_DEFINES := IMAGE_$(4) $($(4)_DEFINES) $(PLAT_BL_COMMON_DEFINES)) +$(eval BL_INCLUDE_DIRS := $($(4)_INCLUDE_DIRS) $(PLAT_BL_COMMON_INCLUDE_DIRS)) +$(eval BL_CPPFLAGS := $($(4)_CPPFLAGS) $(addprefix -D,$(BL_DEFINES)) $(addprefix -I,$(BL_INCLUDE_DIRS)) $(PLAT_BL_COMMON_CPPFLAGS)) +$(eval BL_ASFLAGS := $($(4)_ASFLAGS) $(PLAT_BL_COMMON_ASFLAGS)) $(OBJ): $(2) $(filter-out %.d,$(MAKEFILE_LIST)) | $$$$(@D)/ $$(s)echo " AS $$<" @@ -364,13 +369,14 @@ endef # $(1) = output linker script # $(2) = input template # $(3) = BL stage +# $(4) = uppercase BL stage define MAKE_LD $(eval DEP := $(1).d) -$(eval BL_DEFINES := IMAGE_$(call uppercase,$(3)) $($(call uppercase,$(3))_DEFINES) $(PLAT_BL_COMMON_DEFINES)) -$(eval BL_INCLUDE_DIRS := $($(call uppercase,$(3))_INCLUDE_DIRS) $(PLAT_BL_COMMON_INCLUDE_DIRS)) -$(eval BL_CPPFLAGS := $($(call uppercase,$(3))_CPPFLAGS) $(addprefix -D,$(BL_DEFINES)) $(addprefix -I,$(BL_INCLUDE_DIRS)) $(PLAT_BL_COMMON_CPPFLAGS)) +$(eval BL_DEFINES := IMAGE_$(4) $($(4)_DEFINES) $(PLAT_BL_COMMON_DEFINES)) +$(eval BL_INCLUDE_DIRS := $($(4)_INCLUDE_DIRS) $(PLAT_BL_COMMON_INCLUDE_DIRS)) +$(eval BL_CPPFLAGS := $($(4)_CPPFLAGS) $(addprefix -D,$(BL_DEFINES)) $(addprefix -I,$(BL_INCLUDE_DIRS)) $(PLAT_BL_COMMON_CPPFLAGS)) $(1): $(2) $(filter-out %.d,$(MAKEFILE_LIST)) | $$$$(@D)/ $$(s)echo " PP $$<" @@ -384,14 +390,15 @@ endef # $(1) = output directory # $(2) = list of source files # $(3) = name of the library +# $(4) = uppercase name of the library define MAKE_LIB_OBJS $(eval C_OBJS := $(filter %.c,$(2))) $(eval REMAIN := $(filter-out %.c,$(2))) - $(eval $(foreach obj,$(C_OBJS),$(call MAKE_C_LIB,$(1),$(obj),$(3)))) + $(eval $(foreach obj,$(C_OBJS),$(call MAKE_C_LIB,$(1),$(obj),$(3),$(4)))) $(eval S_OBJS := $(filter %.S,$(REMAIN))) $(eval REMAIN := $(filter-out %.S,$(REMAIN))) - $(eval $(foreach obj,$(S_OBJS),$(call MAKE_S_LIB,$(1),$(obj),$(3)))) + $(eval $(foreach obj,$(S_OBJS),$(call MAKE_S_LIB,$(1),$(obj),$(3),$(4)))) $(and $(REMAIN),$(error Unexpected source files present: $(REMAIN))) endef @@ -401,14 +408,15 @@ endef # $(1) = output directory # $(2) = list of source files (both C and assembly) # $(3) = BL stage +# $(4) = uppercase BL stage define MAKE_OBJS $(eval C_OBJS := $(filter %.c,$(2))) $(eval REMAIN := $(filter-out %.c,$(2))) - $(eval $(foreach obj,$(C_OBJS),$(call MAKE_C,$(1),$(obj),$(3)))) + $(eval $(foreach obj,$(C_OBJS),$(call MAKE_C,$(1),$(obj),$(3),$(4)))) $(eval S_OBJS := $(filter %.S,$(REMAIN))) $(eval REMAIN := $(filter-out %.S,$(REMAIN))) - $(eval $(foreach obj,$(S_OBJS),$(call MAKE_S,$(1),$(obj),$(3)))) + $(eval $(foreach obj,$(S_OBJS),$(call MAKE_S,$(1),$(obj),$(3),$(4)))) $(and $(REMAIN),$(error Unexpected source files present: $(REMAIN))) endef @@ -428,13 +436,14 @@ endef # Arguments: # $(1) = Library name define MAKE_LIB + $(eval BL := $(call uppercase,$(1))) $(eval BUILD_DIR := ${BUILD_PLAT}/lib$(1)) $(eval LIB_DIR := ${BUILD_PLAT}/lib) $(eval ROMLIB_DIR := ${BUILD_PLAT}/romlib) - $(eval SOURCES := $(LIB$(call uppercase,$(1))_SRCS)) + $(eval SOURCES := $(LIB$(BL)_SRCS)) $(eval OBJS := $(addprefix $(BUILD_DIR)/,$(call SOURCES_TO_OBJS,$(SOURCES)))) -$(eval $(call MAKE_LIB_OBJS,$(BUILD_DIR),$(SOURCES),$(1))) +$(eval $(call MAKE_LIB_OBJS,$(BUILD_DIR),$(SOURCES),$(1),$(BL))) libraries: ${LIB_DIR}/lib$(1).a ifeq ($($(ARCH)-ld-id),arm-link) @@ -476,8 +485,9 @@ endif # $(3) = FIP prefix (optional) (if FWU_, target is fwu_fip instead of fip) # $(4) = BL encryption flag (optional) (0, 1) define MAKE_BL + $(eval BL := $(call uppercase,$(1))) $(eval BUILD_DIR := ${BUILD_PLAT}/$(1)) - $(eval BL_SOURCES := $($(call uppercase,$(1))_SOURCES)) + $(eval BL_SOURCES := $($(BL)_SOURCES)) $(eval SOURCES := $(sort $(BL_SOURCES) $(BL_COMMON_SOURCES) $(PLAT_BL_COMMON_SOURCES))) $(eval OBJS := $(addprefix $(BUILD_DIR)/,$(call SOURCES_TO_OBJS,$(SOURCES)))) $(eval MAPFILE := $(call IMG_MAPFILE,$(1))) @@ -485,21 +495,21 @@ define MAKE_BL $(eval DUMP := $(call IMG_DUMP,$(1))) $(eval BIN := $(call IMG_BIN,$(1))) $(eval ENC_BIN := $(call IMG_ENC_BIN,$(1))) - $(eval BL_LIBS := $($(call uppercase,$(1))_LIBS)) + $(eval BL_LIBS := $($(BL)_LIBS)) - $(eval DEFAULT_LINKER_SCRIPT_SOURCE := $($(call uppercase,$(1))_DEFAULT_LINKER_SCRIPT_SOURCE)) + $(eval DEFAULT_LINKER_SCRIPT_SOURCE := $($(BL)_DEFAULT_LINKER_SCRIPT_SOURCE)) $(eval DEFAULT_LINKER_SCRIPT := $(call linker_script_path,$(DEFAULT_LINKER_SCRIPT_SOURCE))) - $(eval LINKER_SCRIPT_SOURCES := $($(call uppercase,$(1))_LINKER_SCRIPT_SOURCES)) + $(eval LINKER_SCRIPT_SOURCES := $($(BL)_LINKER_SCRIPT_SOURCES)) $(eval LINKER_SCRIPTS := $(call linker_script_path,$(LINKER_SCRIPT_SOURCES))) -$(eval $(call MAKE_OBJS,$(BUILD_DIR),$(SOURCES),$(1))) +$(eval $(call MAKE_OBJS,$(BUILD_DIR),$(SOURCES),$(1),$(BL))) # Generate targets to preprocess each required linker script $(eval $(foreach source,$(DEFAULT_LINKER_SCRIPT_SOURCE) $(LINKER_SCRIPT_SOURCES), \ - $(call MAKE_LD,$(call linker_script_path,$(source)),$(source),$(1)))) + $(call MAKE_LD,$(call linker_script_path,$(source)),$(source),$(1),$(BL)))) -$(eval BL_LDFLAGS := $($(call uppercase,$(1))_LDFLAGS)) +$(eval BL_LDFLAGS := $($(BL)_LDFLAGS)) ifeq ($(USE_ROMLIB),1) $(ELF): romlib.bin | $$$$(@D)/ @@ -554,10 +564,10 @@ all: $(1) ifeq ($(4),1) $(call ENCRYPT_FW,$(BIN),$(ENC_BIN)) -$(if $(2),$(call TOOL_ADD_IMG_PAYLOAD,$(1),$(BIN),--$(2),$(ENC_BIN),$(3), \ +$(if $(2),$(call TOOL_ADD_IMG_PAYLOAD,$(BL),$(BIN),--$(2),$(ENC_BIN),$(3), \ $(ENC_BIN))) else -$(if $(2),$(call TOOL_ADD_IMG_PAYLOAD,$(1),$(BIN),--$(2),$(BIN),$(3))) +$(if $(2),$(call TOOL_ADD_IMG_PAYLOAD,$(BL),$(BIN),--$(2),$(BIN),$(3))) endif endef diff --git a/make_helpers/common.mk b/make_helpers/common.mk index 75d9f7179..848e4e944 100644 --- a/make_helpers/common.mk +++ b/make_helpers/common.mk @@ -9,9 +9,9 @@ ifndef common-mk include $(dir $(common-mk))utilities.mk - silent = $(call bool,$(findstring s,$(firstword ~$(MAKEFLAGS)))) - verbose = $(if $(silent),,$(call bool,$(V))) + silent := $(call bool,$(findstring s,$(firstword ~$(MAKEFLAGS)))) + verbose := $(if $(silent),,$(call bool,$(V))) - s = @$(if $(or $(verbose),$(silent)),: ) - q = $(if $(verbose),,@) + s := @$(if $(or $(verbose),$(silent)),: ) + q := $(if $(verbose),,@) endif diff --git a/plat/arm/board/arm_fpga/platform.mk b/plat/arm/board/arm_fpga/platform.mk index c1dc5f5a0..967bf2171 100644 --- a/plat/arm/board/arm_fpga/platform.mk +++ b/plat/arm/board/arm_fpga/platform.mk @@ -128,9 +128,9 @@ BL31_SOURCES += common/fdt_fixup.c \ BL31_SOURCES += ${FDT_WRAPPERS_SOURCES} -$(eval $(call MAKE_S,$(BUILD_PLAT),plat/arm/board/arm_fpga/rom_trampoline.S,bl31)) -$(eval $(call MAKE_S,$(BUILD_PLAT),plat/arm/board/arm_fpga/kernel_trampoline.S,bl31)) -$(eval $(call MAKE_LD,$(BUILD_PLAT)/build_axf.ld,plat/arm/board/arm_fpga/build_axf.ld.S,bl31)) +$(eval $(call MAKE_S,$(BUILD_PLAT),plat/arm/board/arm_fpga/rom_trampoline.S,bl31,BL31)) +$(eval $(call MAKE_S,$(BUILD_PLAT),plat/arm/board/arm_fpga/kernel_trampoline.S,bl31,BL31)) +$(eval $(call MAKE_LD,$(BUILD_PLAT)/build_axf.ld,plat/arm/board/arm_fpga/build_axf.ld.S,bl31,BL31)) ifeq ($($(ARCH)-ld-id),gnu-gcc) AXF_LDFLAGS += -Wl,--build-id=none -mno-fix-cortex-a53-843419 From 316f5c97f29ed553e14da5cd60c1d989a9369897 Mon Sep 17 00:00:00 2001 From: Boyan Karatotev Date: Thu, 10 Oct 2024 13:54:37 +0100 Subject: [PATCH 2/2] perf(build): don't check the compiler's flags for every target The TF_FLAGS variable must be recursively expanded as the rules that use it are defined before it has been fully defined. That has the unfortunate side effect of spawning a subshell that calls the compiler for every file that is being built, thrashing multicore build times. We don't cater to the possibility of the toolchain changing mid build so precomputing this value would be more sensible. Doing a clean build on an Intel dual socket Xeon Gold 5218 (i.e. 64 threads) workstation used to take about 9 seconds. After this patch it takes about 1.5. Single core performance went from ~45 seconds to about 25. Change-Id: If56ed0ab3cc42bc482d9dd05a41ffbff4dd7f147 Signed-off-by: Boyan Karatotev --- Makefile | 6 ++++-- make_helpers/build_macros.mk | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 6bbca3bfd..960befc2d 100644 --- a/Makefile +++ b/Makefile @@ -256,10 +256,12 @@ WARNINGS += -Wunused-but-set-variable -Wmaybe-uninitialized \ -Wlogical-op # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523 -TF_CFLAGS += $(call cc_option, --param=min-pagesize=0) +TF_CFLAGS_MIN_PAGE_SIZE := $(call cc_option, --param=min-pagesize=0) +TF_CFLAGS += $(TF_CFLAGS_MIN_PAGE_SIZE) ifeq ($(HARDEN_SLS), 1) - TF_CFLAGS_aarch64 += $(call cc_option, -mharden-sls=all) + TF_CFLAGS_MHARDEN_SLS := $(call cc_option, -mharden-sls=all) + TF_CFLAGS_aarch64 += $(TF_CFLAGS_MHARDEN_SLS) endif else diff --git a/make_helpers/build_macros.mk b/make_helpers/build_macros.mk index b6e64219f..d454efd90 100644 --- a/make_helpers/build_macros.mk +++ b/make_helpers/build_macros.mk @@ -96,6 +96,10 @@ ld_option = $(shell $($(ARCH)-ld) $(1) -Wl,--version >/dev/null 2>&1 || $($(ARCH # Convenience function to check for a given compiler option. A call to # $(call cc_option, --no-XYZ) will return --no-XYZ if supported by the compiler +# NOTE: consider assigning to an immediately expanded temporary variable before +# assigning. This is because variables like TF_CFLAGS are recursively expanded +# and assigning this directly will cause it to be expanded every time the +# variable is used, potentially thrashing multicore performance. define cc_option $(shell if $($(ARCH)-cc) $(1) -c -x c /dev/null -o /dev/null >/dev/null 2>&1; then echo $(1); fi ) endef