8326375: [REDO] Clean up NativeCompilation.gmk and its newly created …

…parts Reviewed-by: jwaters, erikj
openjdk · Mar 6, 2024 · 8273221 · 8273221
1 parent 8a05459
commit 8273221
Show file tree

Hide file tree

Showing 6 changed files with 252 additions and 247 deletions.
diff --git a/make/common/NativeCompilation.gmk b/make/common/NativeCompilation.gmk
@@ -25,8 +25,8 @@
 
 ################################################################################
 # This is the top-level entry point for our native compilation and linking.
-# It contains the SetupNativeCompilation function, but is supported by helper
-# functions in the make/common/native directory.
+# It contains the SetupNativeCompilation macro, but is supported by helper
+# macros in the make/common/native directory.
 ################################################################################
 
 ifndef _NATIVE_COMPILATION_GMK
@@ -129,23 +129,25 @@ include native/ToolchainDefinitions.gmk
 #
 SetupNativeCompilation = $(NamedParamsMacroTemplate)
 define SetupNativeCompilationBody
-  # Setup variables for the rest of the function to work with
-  $$(eval $$(call SetupBasicVariables1,$1))
-  $$(eval $$(call SetupDebugSymbols,$1))
-  $$(eval $$(call SetupBasicVariables2,$1))
+  # When reading this code, note that macros named Setup<Foo> are just setting
+  # variables, and macros called Create<Foo> are setting up rules to create
+  # files. Macros starting with any other verb are more complicated, and can do
+  # all of the above, and also call directly to the shell.
+
+  ###
+  ### Prepare for compilation and linking
+  ###
+
   $$(eval $$(call VerifyArguments,$1))
-  $$(eval $$(call SetupBasicVariables3,$1))
 
-  # Need to make sure TARGET is first on list
-  $1 := $$($1_TARGET)
+  # Setup variables for the rest of this macro to work with
+  $$(eval $$(call SetupBasicVariables,$1))
 
   # Setup the toolchain to be used
   $$(eval $$(call SetupToolchain,$1))
 
-  # Figure out all source files to compile
-  $$(eval $$(call LocateSourceFiles,$1))
-
-  # ... and what the output object files will be
+  # Find all source files to compile and determine the output object file names
+  $$(eval $$(call SetupSourceFiles,$1))
   $$(eval $$(call SetupOutputFiles,$1))
 
   # Setup CFLAGS/CXXFLAGS based on warnings, optimizations, extra flags etc.
@@ -154,20 +156,29 @@ define SetupNativeCompilationBody
   # Machinery needed for the build to function properly
   $$(eval $$(call SetupBuildSystemSupport,$1))
 
+  $$(eval $$(call RemoveSuperfluousOutputFiles,$1))
+
+  # Need to make sure TARGET is first on list before starting to create files
+  $1 := $$($1_TARGET)
+
+  # Have make print information about the library when we start compiling
+  $$(eval $$(call PrintStartInfo,$1))
+
+  ###
+  ### Compile all native source code files
+  ###
+
   # Create a PCH, if requested
   $$(eval $$(call CreatePrecompiledHeader,$1))
 
-  # Now call SetupCompileNativeFile for each source file we are going to compile.
+  # Now call CreateCompiledNativeFile for each source file we are going to compile.
   $$(foreach file, $$($1_SRCS), \
-      $$(eval $$(call SetupCompileNativeFile,$1_$$(notdir $$(file)),\
+      $$(eval $$(call CreateCompiledNativeFile,$1_$$(notdir $$(file)),\
           FILE := $$(file), \
           BASE := $1, \
       )) \
   )
 
-  # Have make print information about the library when we start compiling
-  $$(eval $$(call PrintInfo,$1))
-
   ifeq ($(call isTargetOs, windows), true)
     # On windows we need to create a resource file
     $$(eval $$(call CreateWindowsResourceFile,$1))
@@ -178,15 +189,16 @@ define SetupNativeCompilationBody
   $$(eval $$(call CreateDependencyFile,$1))
   $$(eval $$(call ImportDependencyFile,$1))
 
-  # Prepare for linking
-  $$(eval $$(call SetupLinkerFlags,$1))
-
-  $$(eval $$(call SetupMapfile,$1))
+  ###
+  ### Link the object files into a native output library/executable
+  ###
 
   # Handle native debug symbols
   $$(eval $$(call CreateDebugSymbols,$1))
 
-  $$(eval $$(call SetupStrip,$1))
+  # Prepare for linking
+  $$(eval $$(call SetupLinkerFlags,$1))
+  $$(eval $$(call SetupLinking,$1))
 
   $$(eval $$(call SetupObjectFileList,$1))
 
@@ -200,8 +212,30 @@ define SetupNativeCompilationBody
 endef
 
 ################################################################################
-# Setup basic variables, part 1
-define SetupBasicVariables1
+# Verify that user passed arguments are valid
+define VerifyArguments
+  ifneq ($$($1_NAME), $(basename $$($1_NAME)))
+    $$(error NAME must not contain any directory path in $1)
+  endif
+  ifneq ($(findstring $$($1_SUFFIX), $$($1_NAME)), )
+    $$(error NAME should be specified without suffix: $$($1_SUFFIX) in $1)
+  endif
+  ifneq ($(findstring $$($1_PREFIX), $$($1_NAME)), )
+    $$(error NAME should be specified without prefix: $$($1_PREFIX) in $1)
+  endif
+  ifeq ($$($1_OUTPUT_DIR), )
+    $$(error OUTPUT_DIR is missing in $1)
+  endif
+  ifneq ($$($1_MANIFEST), )
+    ifeq ($$($1_MANIFEST_VERSION), )
+      $$(error If MANIFEST is provided, then MANIFEST_VERSION is required in $1)
+    endif
+  endif
+endef
+
+################################################################################
+# Setup basic variables
+define SetupBasicVariables
   # If type is unspecified, default to LIBRARY
   ifeq ($$($1_TYPE), )
     $1_TYPE := LIBRARY
@@ -214,11 +248,7 @@ define SetupBasicVariables1
       $1_TYPE := STATIC_LIBRARY
     endif
   endif
-endef
 
-################################################################################
-# Setup basic variables, part 2
-define SetupBasicVariables2
   # STATIC_LIBS is set from Main.gmk when building static versions of certain
   # native libraries.
   ifeq ($(STATIC_LIBS), true)
@@ -247,33 +277,7 @@ define SetupBasicVariables2
       endif
     endif
   endif
-endef
-
-################################################################################
-# Verify that user passed arguments are valid
-define VerifyArguments
-  ifneq ($$($1_NAME), $(basename $$($1_NAME)))
-    $$(error NAME must not contain any directory path in $1)
-  endif
-  ifneq ($(findstring $$($1_SUFFIX), $$($1_NAME)), )
-    $$(error NAME should be specified without suffix: $$($1_SUFFIX) in $1)
-  endif
-  ifneq ($(findstring $$($1_PREFIX), $$($1_NAME)), )
-    $$(error NAME should be specified without prefix: $$($1_PREFIX) in $1)
-  endif
-  ifeq ($$($1_OUTPUT_DIR), )
-    $$(error OUTPUT_DIR is missing in $1)
-  endif
-  ifneq ($$($1_MANIFEST), )
-    ifeq ($$($1_MANIFEST_VERSION), )
-      $$(error If MANIFEST is provided, then MANIFEST_VERSION is required in $1)
-    endif
-  endif
-endef
 
-################################################################################
-# Setup basic variables, part 3
-define SetupBasicVariables3
   $1_BASENAME := $$($1_PREFIX)$$($1_NAME)$$($1_SUFFIX)
   $1_TARGET := $$($1_OUTPUT_DIR)/$$($1_BASENAME)
   $1_NOSUFFIX := $$($1_PREFIX)$$($1_NAME)
@@ -283,8 +287,6 @@ endef
 ################################################################################
 # Setup machinery needed by the build system
 define SetupBuildSystemSupport
-  $1_BUILD_INFO := $$($1_OBJECT_DIR)/_build-info.marker
-
   # Track variable changes for all variables that affect the compilation command
   # lines for all object files in this setup. This includes at least all the
   # variables used in the call to add_native_source below.
@@ -297,13 +299,15 @@ endef
 
 ################################################################################
 # Have make print information about the library when we start compiling
-define PrintInfo
+define PrintStartInfo
   # Setup rule for printing progress info when compiling source files.
   # This is a rough heuristic and may not always print accurate information.
   # The $1_BUILD_INFO and $1_BUILD_INFO_DEPS variables are used in
   # TestFilesCompilation.gmk.
   $$(call SetIfEmpty, $1_BUILD_INFO_LOG_MACRO, LogWarn)
   $1_BUILD_INFO_DEPS := $$($1_SRCS) $$($1_COMPILE_VARDEPS_FILE)
+  $1_BUILD_INFO := $$($1_OBJECT_DIR)/_build-info.marker
+
   $$($1_BUILD_INFO): $$($1_BUILD_INFO_DEPS)
         ifeq ($$(wildcard $$($1_TARGET)), )
 	  $$(call $$($1_BUILD_INFO_LOG_MACRO), \

diff --git a/make/common/native/CompileFile.gmk b/make/common/native/CompileFile.gmk
@@ -100,8 +100,8 @@ DEPENDENCY_TARGET_SED_PATTERN := \
 #   FILE - The full path of the source file to compiler
 #   BASE - The name of the rule for the entire binary to build ($1)
 #
-SetupCompileNativeFile = $(NamedParamsMacroTemplate)
-define SetupCompileNativeFileBody
+CreateCompiledNativeFile = $(NamedParamsMacroTemplate)
+define CreateCompiledNativeFileBody
   $1_FILENAME := $$(notdir $$($1_FILE))
 
   # The target file to be generated.
@@ -120,7 +120,7 @@ define SetupCompileNativeFileBody
     # This is the definite source file to use for $1_FILENAME.
     $1_SRC_FILE := $$($1_FILE)
 
-    $$(eval $$(call SetupCompileFileFlags,$1))
+    $$(eval $$(call SetupCompileFileFlags,$1,$$($1_BASE)))
 
     ifneq ($$(filter %.c, $$($1_FILENAME)), )
       # Compile as a C file
@@ -250,7 +250,7 @@ define CreatePrecompiledHeader
         $1_GENERATED_PCH_SRC := $$($1_OBJECT_DIR)/$1_pch.cpp
         $1_GENERATED_PCH_OBJ := $$($1_OBJECT_DIR)/$1_pch$(OBJ_SUFFIX)
 
-        $$(eval $$(call SetupCompileNativeFile, $1_$$(notdir $$($1_GENERATED_PCH_SRC)), \
+        $$(eval $$(call CreateCompiledNativeFile, $1_$$(notdir $$($1_GENERATED_PCH_SRC)), \
             FILE := $$($1_GENERATED_PCH_SRC), \
             BASE := $1, \
             EXTRA_CXXFLAGS := -Fp$$($1_PCH_FILE) -Yc$$(notdir $$($1_PRECOMPILED_HEADER)), \

diff --git a/make/common/native/DebugSymbols.gmk b/make/common/native/DebugSymbols.gmk
@@ -26,23 +26,6 @@
 ################################################################################
 # This file contains functionality related to native debug symbol handling.
 
-################################################################################
-define SetupDebugSymbols
-  $$(call SetIfEmpty, $1_COMPILE_WITH_DEBUG_SYMBOLS, $$(COMPILE_WITH_DEBUG_SYMBOLS))
-
-  ifeq ($(STATIC_LIBS), true)
-    # For release builds where debug symbols are configured to be moved to
-    # separate debuginfo files, disable debug symbols for static libs instead.
-    # We don't currently support this configuration and we don't want symbol
-    # information in release builds unless explicitly asked to provide it.
-    ifeq ($(DEBUG_LEVEL), release)
-      ifeq ($(COPY_DEBUG_SYMBOLS), true)
-        $1_COMPILE_WITH_DEBUG_SYMBOLS := false
-      endif
-    endif
-  endif
-endef
-
 ################################################################################
 define CreateDebugSymbols
   ifneq ($$($1_COPY_DEBUG_SYMBOLS), false)

diff --git a/make/common/native/Flags.gmk b/make/common/native/Flags.gmk
@@ -29,10 +29,12 @@
 # like optimization level.
 
 ################################################################################
+# $1 is the prefix of the file to be compiled
+# $2 is the prefix of the library, i.e. $$($1_BASE)
 define SetupCompileFileFlags
   ifeq ($$($1_OPTIMIZATION), )
-    $1_OPT_CFLAGS := $$($$($1_BASE)_OPT_CFLAGS)
-    $1_OPT_CXXFLAGS := $$($$($1_BASE)_OPT_CXXFLAGS)
+    $1_OPT_CFLAGS := $$($2_OPT_CFLAGS)
+    $1_OPT_CXXFLAGS := $$($2_OPT_CXXFLAGS)
   else
     ifeq ($$($1_OPTIMIZATION), NONE)
       $1_OPT_CFLAGS := $(C_O_FLAG_NONE)
@@ -57,23 +59,23 @@ define SetupCompileFileFlags
     endif
   endif
 
-  ifneq ($$($$($1_BASE)_PRECOMPILED_HEADER), )
-    ifeq ($$(filter $$($1_FILENAME), $$($$($1_BASE)_PRECOMPILED_HEADER_EXCLUDE)), )
-      $1_USE_PCH_FLAGS := $$($$($1_BASE)_USE_PCH_FLAGS)
+  ifneq ($$($2_PRECOMPILED_HEADER), )
+    ifeq ($$(filter $$($1_FILENAME), $$($2_PRECOMPILED_HEADER_EXCLUDE)), )
+      $1_USE_PCH_FLAGS := $$($2_USE_PCH_FLAGS)
     endif
   endif
 
   ifneq ($(DISABLE_WARNING_PREFIX), )
     $1_WARNINGS_FLAGS := $$(addprefix $(DISABLE_WARNING_PREFIX), \
-      $$($$($1_BASE)_DISABLED_WARNINGS_$(TOOLCHAIN_TYPE)_$$($1_FILENAME)) \
-      $$($$($1_BASE)_DISABLED_WARNINGS_$(TOOLCHAIN_TYPE)_$(OPENJDK_TARGET_OS)_$$($1_FILENAME)))
+      $$($2_DISABLED_WARNINGS_$(TOOLCHAIN_TYPE)_$$($1_FILENAME)) \
+      $$($2_DISABLED_WARNINGS_$(TOOLCHAIN_TYPE)_$(OPENJDK_TARGET_OS)_$$($1_FILENAME)))
   endif
 
-  $1_BASE_CFLAGS :=  $$($$($1_BASE)_CFLAGS) $$($$($1_BASE)_EXTRA_CFLAGS) \
-      $$($$($1_BASE)_SYSROOT_CFLAGS)
-  $1_BASE_CXXFLAGS := $$($$($1_BASE)_CXXFLAGS) $$($$($1_BASE)_EXTRA_CXXFLAGS) \
-      $$($$($1_BASE)_SYSROOT_CFLAGS) $$($1_EXTRA_CXXFLAGS)
-  $1_BASE_ASFLAGS := $$($$($1_BASE)_ASFLAGS) $$($$($1_BASE)_EXTRA_ASFLAGS)
+  $1_BASE_CFLAGS :=  $$($2_CFLAGS) $$($2_EXTRA_CFLAGS) \
+      $$($2_SYSROOT_CFLAGS)
+  $1_BASE_CXXFLAGS := $$($2_CXXFLAGS) $$($2_EXTRA_CXXFLAGS) \
+      $$($2_SYSROOT_CFLAGS) $$($1_EXTRA_CXXFLAGS)
+  $1_BASE_ASFLAGS := $$($2_ASFLAGS) $$($2_EXTRA_ASFLAGS)
 endef
 
 ################################################################################
@@ -129,6 +131,20 @@ define SetupCompilerFlags
     $1_EXTRA_CXXFLAGS := $$($1_EXTRA_CFLAGS)
   endif
 
+  $$(call SetIfEmpty, $1_COMPILE_WITH_DEBUG_SYMBOLS, $$(COMPILE_WITH_DEBUG_SYMBOLS))
+
+  ifeq ($(STATIC_LIBS), true)
+    # For release builds where debug symbols are configured to be moved to
+    # separate debuginfo files, disable debug symbols for static libs instead.
+    # We don't currently support this configuration and we don't want symbol
+    # information in release builds unless explicitly asked to provide it.
+    ifeq ($(DEBUG_LEVEL), release)
+      ifeq ($(COPY_DEBUG_SYMBOLS), true)
+        $1_COMPILE_WITH_DEBUG_SYMBOLS := false
+      endif
+    endif
+  endif
+
   ifeq ($$($1_COMPILE_WITH_DEBUG_SYMBOLS), true)
     $1_EXTRA_CFLAGS += $$(CFLAGS_DEBUG_SYMBOLS)
     $1_EXTRA_CXXFLAGS += $$(CFLAGS_DEBUG_SYMBOLS)