From 50411883a9f6c751bdcdbe6890ce5338d722e082 Mon Sep 17 00:00:00 2001 From: Mitchell O'Sullivan Date: Tue, 30 May 2023 01:04:52 +1000 Subject: [PATCH 1/4] Improve modularity of GCC compatibility resolution This is a minor update that uses more sophisticated methods to validate the supported compiler arguments. --- Makefile | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 52a1c60d..7f5293c0 100644 --- a/Makefile +++ b/Makefile @@ -16,11 +16,17 @@ BENCHS = $(wildcard $(BENCHDIR)/*.cpp) TESTOBJS = $(patsubst $(TESTDIR)/%.cpp,$(TESTDIR)/%.o,$(TESTS)) BENCHOBJS = $(patsubst $(BENCHDIR)/%.cpp,$(BENCHDIR)/%.o,$(BENCHS)) -# Compiling AVX512-FP16 instructions isn't possible for g++ < 12 -ifeq ($(shell expr `$(CXX) -dumpversion | cut -d '.' -f 1` \< 12), 1) - MARCHFLAG = -march=icelake-client - BENCHOBJS_SKIP += bench-qsortfp16.o - TESTOBJS_SKIP += test-qsortfp16.o +test_cxx_flag = $(shell 2>/dev/null $(CXX) -o /dev/null $(1) -c -x c++ /dev/null; echo $$?) + +# Compiling AVX512-FP16 instructions wasn't possible until GCC 12 +ifeq ($(call test_cxx_flag,-mavx512fp16), 1) + BENCHOBJS_SKIP += bench-qsortfp16.o + TESTOBJS_SKIP += test-qsortfp16.o +endif + +# Sapphire Rapids was otherwise supported from GCC 11. Downgrade if required. +ifeq ($(call test_cxx_flag,$(MARCHFLAG)), 1) + MARCHFLAG = -march=icelake-client endif BENCHOBJS := $(filter-out $(addprefix $(BENCHDIR)/, $(BENCHOBJS_SKIP)) ,$(BENCHOBJS)) From d0623a731fd28dbec52564d646db3e7960663afa Mon Sep 17 00:00:00 2001 From: Mitchell O'Sullivan Date: Tue, 27 Jun 2023 02:35:22 +1000 Subject: [PATCH 2/4] Substantially update Makefile to improve coverage The header files are now considered when make decides what targets need updating. Configured .DEFAULT_GOAL and .PHONY for the non-file targets. Tweaked file to predominantly rely on the default implicit rules for building the `.o` objects (read: moved all the options into CXXFLAGS). In addition to tracking with each `.hpp` or `.h` file inside of tests and benchmarks (accordingly), each `.o` object will also trigger a rebuild when there are changes to the main source files in src. However, each `.cpp` is otherwise independent of each other. --- .gitignore | 2 ++ Makefile | 88 ++++++++++++++++++++++++++++++++---------------------- 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/.gitignore b/.gitignore index adb72083..eccedaff 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,5 @@ # Build or IDE artifacts **/.vscode /builddir/ +/testexe +/benchexe diff --git a/Makefile b/Makefile index 7f5293c0..6c4b833a 100644 --- a/Makefile +++ b/Makefile @@ -1,57 +1,75 @@ CXX ?= g++-12 -CXXFLAGS += -I$(SRCDIR) -I$(UTILS) -O3 -GTESTCFLAGS = `pkg-config --cflags gtest_main` -GTESTLDFLAGS = `pkg-config --static --libs gtest_main` -GBENCHCFLAGS = `pkg-config --cflags benchmark` -GBENCHLDFLAGS = `pkg-config --static --libs benchmark` -MARCHFLAG = -march=sapphirerapids - -SRCDIR = ./src -TESTDIR = ./tests -BENCHDIR = ./benchmarks -UTILS = ./utils -SRCS = $(wildcard $(SRCDIR)/*.hpp) -TESTS = $(wildcard $(TESTDIR)/*.cpp) -BENCHS = $(wildcard $(BENCHDIR)/*.cpp) -TESTOBJS = $(patsubst $(TESTDIR)/%.cpp,$(TESTDIR)/%.o,$(TESTS)) -BENCHOBJS = $(patsubst $(BENCHDIR)/%.cpp,$(BENCHDIR)/%.o,$(BENCHS)) - -test_cxx_flag = $(shell 2>/dev/null $(CXX) -o /dev/null $(1) -c -x c++ /dev/null; echo $$?) +CXXFLAGS += $(OPTIMFLAG) $(MARCHFLAG) +override CXXFLAGS += -I$(SRCDIR) -I$(UTILSDIR) +GTESTCFLAGS := `pkg-config --cflags gtest_main` +GTESTLDFLAGS := `pkg-config --static --libs gtest_main` +GBENCHCFLAGS := `pkg-config --cflags benchmark` +GBENCHLDFLAGS := `pkg-config --static --libs benchmark` +OPTIMFLAG := -O3 +MARCHFLAG := -march=sapphirerapids + +SRCDIR := ./src +TESTDIR := ./tests +BENCHDIR := ./benchmarks +UTILSDIR := ./utils + +SRCS := $(wildcard $(addprefix $(SRCDIR)/, *.hpp *.h)) +UTILSRCS := $(wildcard $(addprefix $(UTILSDIR)/, *.hpp *.h)) +TESTSRCS := $(wildcard $(addprefix $(TESTDIR)/, *.hpp *.h)) +BENCHSRCS := $(wildcard $(addprefix $(BENCHDIR)/, *.hpp *.h)) +UTILS := $(wildcard $(UTILSDIR)/*.cpp) +TESTS := $(wildcard $(TESTDIR)/*.cpp) +BENCHS := $(wildcard $(BENCHDIR)/*.cpp) + +test_cxx_flag = $(shell 2>/dev/null $(CXX) -o /dev/null $(1) -c -x c++ /dev/null; echo $$?) # Compiling AVX512-FP16 instructions wasn't possible until GCC 12 ifeq ($(call test_cxx_flag,-mavx512fp16), 1) - BENCHOBJS_SKIP += bench-qsortfp16.o - TESTOBJS_SKIP += test-qsortfp16.o + BENCHS_SKIP += bench-qsortfp16.cpp + TESTS_SKIP += test-qsortfp16.cpp endif # Sapphire Rapids was otherwise supported from GCC 11. Downgrade if required. ifeq ($(call test_cxx_flag,$(MARCHFLAG)), 1) - MARCHFLAG = -march=icelake-client + MARCHFLAG := -march=icelake-client endif -BENCHOBJS := $(filter-out $(addprefix $(BENCHDIR)/, $(BENCHOBJS_SKIP)) ,$(BENCHOBJS)) -TESTOBJS := $(filter-out $(addprefix $(TESTDIR)/, $(TESTOBJS_SKIP)) ,$(TESTOBJS)) +BENCHOBJS := $(patsubst %.cpp, %.o, $(filter-out $(addprefix $(BENCHDIR)/, $(BENCHS_SKIP)), $(BENCHS))) +TESTOBJS := $(patsubst %.cpp, %.o, $(filter-out $(addprefix $(TESTDIR)/, $(TESTS_SKIP)), $(TESTS))) +UTILOBJS := $(UTILS:.cpp=.o) + +# Stops make from wondering if it needs to generate the .hpp files (.cpp and .h have equivalent rules by default) +%.hpp: + +.PHONY: all +.DEFAULT_GOAL := all +all: test bench + +.PHONY: test +test: testexe -all : test bench +.PHONY: bench +bench: benchexe -$(UTILS)/cpuinfo.o : $(UTILS)/cpuinfo.cpp - $(CXX) $(CXXFLAGS) -c $(UTILS)/cpuinfo.cpp -o $(UTILS)/cpuinfo.o +$(UTILOBJS): $(UTILSRCS) -$(TESTDIR)/%.o : $(TESTDIR)/%.cpp $(SRCS) - $(CXX) $(CXXFLAGS) $(MARCHFLAG) $(GTESTCFLAGS) -c $< -o $@ +$(TESTOBJS): $(TESTSRCS) $(UTILSRCS) $(SRCS) +$(TESTDIR)/%.o: override CXXFLAGS += $(GTESTCFLAGS) -test: $(TESTOBJS) $(UTILS)/cpuinfo.o $(SRCS) - $(CXX) $(TESTOBJS) $(UTILS)/cpuinfo.o $(MARCHFLAG) $(CXXFLAGS) -lgtest_main $(GTESTLDFLAGS) -o testexe +testexe: $(TESTOBJS) $(UTILOBJS) + $(CXX) $(CXXFLAGS) $^ $(LDLIBS) $(LDFLAGS) -lgtest_main $(GTESTLDFLAGS) -o $@ -$(BENCHDIR)/%.o : $(BENCHDIR)/%.cpp $(SRCS) - $(CXX) $(CXXFLAGS) $(MARCHFLAG) $(GBENCHCFLAGS) -c $< -o $@ +$(BENCHOBJS): $(BENCHSRCS) $(UTILSRCS) $(SRCS) +$(BENCHDIR)/%.o: override CXXFLAGS += $(GBENCHCFLAGS) -bench: $(BENCHOBJS) $(UTILS)/cpuinfo.o - $(CXX) $(BENCHOBJS) $(UTILS)/cpuinfo.o $(MARCHFLAG) $(CXXFLAGS) -lbenchmark_main $(GBENCHLDFLAGS) -o benchexe +benchexe: $(BENCHOBJS) $(UTILOBJS) + $(CXX) $(CXXFLAGS) $^ $(LDLIBS) $(LDFLAGS) -lbenchmark_main $(GBENCHLDFLAGS) -o $@ +.PHONY: meson meson: meson setup --warnlevel 0 --buildtype plain builddir cd builddir && ninja +.PHONY: clean clean: - $(RM) -rf $(TESTDIR)/*.o $(BENCHDIR)/*.o $(UTILS)/*.o testexe benchexe builddir + $(RM) -rf $(TESTOBJS) $(BENCHOBJS) $(UTILOBJS) testexe benchexe builddir From 18d036f4c0f07064cf436037b2da3b6003205cff Mon Sep 17 00:00:00 2001 From: Mitchell O'Sullivan Date: Tue, 27 Jun 2023 21:03:22 +1000 Subject: [PATCH 3/4] Robustify CXX detection logic The first line wasn't doing anything previously because CXX is defined as `g++` by default. This change means that now when CXX is undefined or empty it can be detected automatically. Additionally, the variable is marked as exported so that the detected version will be passed on to meson if it is being invoked via make. --- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6c4b833a..8ea9f6cd 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,9 @@ -CXX ?= g++-12 +# When unset, discover g++. Prioritise the latest version on the path. +ifeq (, $(and $(strip $(CXX)), $(filter-out default undefined, $(origin CXX)))) + override CXX := $(shell basename `which g++-12 g++-11 g++-10 g++-9 g++-8 g++ | head -n 1`) +endif + +export CXX CXXFLAGS += $(OPTIMFLAG) $(MARCHFLAG) override CXXFLAGS += -I$(SRCDIR) -I$(UTILSDIR) GTESTCFLAGS := `pkg-config --cflags gtest_main` From c0a99e068ad3edeed07ef01804fc00955878927a Mon Sep 17 00:00:00 2001 From: Mitchell O'Sullivan Date: Thu, 29 Jun 2023 15:01:16 +1000 Subject: [PATCH 4/4] Silence GCC discovery and bail when none available On some systems, `which` will print to stderr when an argument can't be found. This will now be suppressed. Also removed `basename` to more gracefully handle the situation where `g++` is not on the path. --- Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8ea9f6cd..4474f462 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,9 @@ # When unset, discover g++. Prioritise the latest version on the path. ifeq (, $(and $(strip $(CXX)), $(filter-out default undefined, $(origin CXX)))) - override CXX := $(shell basename `which g++-12 g++-11 g++-10 g++-9 g++-8 g++ | head -n 1`) + override CXX := $(shell which g++-12 g++-11 g++-10 g++-9 g++-8 g++ 2>/dev/null | head -n 1) + ifeq (, $(strip $(CXX))) + $(error Could not locate the g++ compiler. Please manually specify its path using the CXX variable) + endif endif export CXX