[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+icZUVMtrqRA-a3k8N0fVcszm=Sn2Jb9nMfF8mXqvSTu2HUhw@mail.gmail.com>
Date: Tue, 5 Apr 2022 17:46:00 +0200
From: Sedat Dilek <sedat.dilek@...il.com>
To: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
Cc: Nathan Chancellor <nathan@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Fangrui Song <maskray@...gle.com>,
Florian Fainelli <f.fainelli@...il.com>,
Ian Rogers <irogers@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
John Keeping <john@...anate.com>, Leo Yan <leo.yan@...aro.org>,
Michael Petlan <mpetlan@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Build perf with clang, failure with libperf
?
On Tue, Apr 5, 2022 at 4:47 PM Arnaldo Carvalho de Melo
<arnaldo.melo@...il.com> wrote:
>
> Em Mon, Apr 04, 2022 at 02:53:24PM -0700, Nathan Chancellor escreveu:
> > On Mon, Apr 04, 2022 at 05:43:11PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Trying to apply Sedat's patch something changed in my system,
> > > and that patch wasn't enough, so I had to first apply this one:
>
> <SNIP>
>
> > > diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> > > index 1480910c792e2cb3..90774b60d31b2b8e 100644
> > > --- a/tools/build/feature/Makefile
> > > +++ b/tools/build/feature/Makefile
> > > @@ -217,7 +217,7 @@ strip-libs = $(filter-out -l%,$(1))
> > > PERL_EMBED_LDOPTS = $(shell perl -MExtUtils::Embed -e ldopts 2>/dev/null)
> > > PERL_EMBED_LDFLAGS = $(call strip-libs,$(PERL_EMBED_LDOPTS))
> > > PERL_EMBED_LIBADD = $(call grep-libs,$(PERL_EMBED_LDOPTS))
> > > -PERL_EMBED_CCOPTS = `perl -MExtUtils::Embed -e ccopts 2>/dev/null`
> > > +PERL_EMBED_CCOPTS = $(shell perl -MExtUtils::Embed -e ccopts 2>/dev/null)
> > > FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
> > >
> > > $(OUTPUT)test-libperl.bin:
> > >
> > > ----------------------------------------------------- 8< -------------------
>
> <SNIP>
>
> > > So perhaps in this case its better to disable that
> > > -Werror,-Wcompound-token-split-by-macro when building with clang?
>
> > Yes, I think that is probably the best solution. As far as I can tell,
> > at least in this file and context, the warning appears harmless, as the
> > "create a GNU C statement expression from two different macros" is very
> > much intentional, based on the presence of PERL_USE_GCC_BRACE_GROUPS.
> > The warning is fixed in upstream Perl by just avoiding creating GNU C
> > statement expressions using STMT_START and STMT_END:
>
> > https://github.com/Perl/perl5/issues/18780
> > https://github.com/Perl/perl5/pull/18984
> >
> > If I am reading the source code correctly, an alternative to disabling
> > the warning would be specifying -DPERL_GCC_BRACE_GROUPS_FORBIDDEN but it
> > seems like that might end up impacting more than just this site,
> > according to the issue discussion above.
>
> Ok, so I ended up with the patch below.
>
> On the 5.19 window I'll make the tools/perf/Makefile.config filters only
> apply when clang is used and unify these settings so that we have it
> setup just in one place, shared by the main build and the feature build.
>
Hi Arnaldo,
Cool, you stepped in.
Some comments below...
> - Arnaldo
>
> commit 5b5da660aa70e2a01748f602e9bbcd18b162a176
> Author: Arnaldo Carvalho de Melo <acme@...hat.com>
> Date: Tue Apr 5 10:33:21 2022 -0300
>
> tools build: Filter out options and warnings not supported by clang
>
> These make the feature check fail when using clang, so remove them just
> like is done in tools/perf/Makefile.config to build perf itself.
>
> Adding -Wno-compound-token-split-by-macro to tools/perf/Makefile.config
> when building with clang is also necessary to avoid these warnings
> turned into errors (-Werror):
>
> CC /tmp/build/perf/util/scripting-engines/trace-event-perl.o
> In file included from util/scripting-engines/trace-event-perl.c:35:
> In file included from /usr/lib64/perl5/CORE/perl.h:4085:
> In file included from /usr/lib64/perl5/CORE/hv.h:659:
> In file included from /usr/lib64/perl5/CORE/hv_func.h:34:
> In file included from /usr/lib64/perl5/CORE/sbox32_hash.h:4:
> /usr/lib64/perl5/CORE/zaphod32_hash.h:150:5: error: '(' and '{' tokens introducing statement expression appear in different macro expansion contexts [-Werror,-Wcompound-token-split-by-macro]
> ZAPHOD32_SCRAMBLE32(state[0],0x9fade23b);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /usr/lib64/perl5/CORE/zaphod32_hash.h:80:38: note: expanded from macro 'ZAPHOD32_SCRAMBLE32'
> #define ZAPHOD32_SCRAMBLE32(v,prime) STMT_START { \
> ^~~~~~~~~~
> /usr/lib64/perl5/CORE/perl.h:737:29: note: expanded from macro 'STMT_START'
> # define STMT_START (void)( /* gcc supports "({ STATEMENTS; })" */
> ^
> /usr/lib64/perl5/CORE/zaphod32_hash.h:150:5: note: '{' token is here
> ZAPHOD32_SCRAMBLE32(state[0],0x9fade23b);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /usr/lib64/perl5/CORE/zaphod32_hash.h:80:49: note: expanded from macro 'ZAPHOD32_SCRAMBLE32'
> #define ZAPHOD32_SCRAMBLE32(v,prime) STMT_START { \
> ^
> /usr/lib64/perl5/CORE/zaphod32_hash.h:150:5: error: '}' and ')' tokens terminating statement expression appear in different macro expansion contexts [-Werror,-Wcompound-token-split-by-macro]
> ZAPHOD32_SCRAMBLE32(state[0],0x9fade23b);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /usr/lib64/perl5/CORE/zaphod32_hash.h:87:41: note: expanded from macro 'ZAPHOD32_SCRAMBLE32'
> v ^= (v>>23); \
> ^
> /usr/lib64/perl5/CORE/zaphod32_hash.h:150:5: note: ')' token is here
> ZAPHOD32_SCRAMBLE32(state[0],0x9fade23b);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /usr/lib64/perl5/CORE/zaphod32_hash.h:88:3: note: expanded from macro 'ZAPHOD32_SCRAMBLE32'
> } STMT_END
> ^~~~~~~~
> /usr/lib64/perl5/CORE/perl.h:738:21: note: expanded from macro 'STMT_END'
> # define STMT_END )
> ^
>
> Based-on-a-patch-by: Sedat Dilek <sedat.dilek@...il.com>
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Fangrui Song <maskray@...gle.com>
> Cc: Florian Fainelli <f.fainelli@...il.com>
> Cc: Ian Rogers <irogers@...gle.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: John Keeping <john@...anate.com>
> Cc: Leo Yan <leo.yan@...aro.org>
> Cc: Michael Petlan <mpetlan@...hat.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Nathan Chancellor <nathan@...nel.org>
> Cc: Nick Desaulniers <ndesaulniers@...gle.com>
> Link: http://lore.kernel.org/lkml/YktYX2OnLtyobRYD@kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
>
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index 90774b60d31b2b8e..de66e1cc073481c5 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -220,6 +220,13 @@ PERL_EMBED_LIBADD = $(call grep-libs,$(PERL_EMBED_LDOPTS))
> PERL_EMBED_CCOPTS = $(shell perl -MExtUtils::Embed -e ccopts 2>/dev/null)
> FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
>
> +ifeq ($(CC_NO_CLANG), 0)
> + PERL_EMBED_LDOPTS := $(filter-out -specs=%,$(PERL_EMBED_LDOPTS))
> + PERL_EMBED_CCOPTS := $(filter-out -flto=auto -ffat-lto-objects, $(PERL_EMBED_CCOPTS))
^^ This affects CONFIG_LTO_CLANG=y only, so check for it?
> + PERL_EMBED_CCOPTS := $(filter-out -specs=%,$(PERL_EMBED_CCOPTS))
> + FLAGS_PERL_EMBED += -Wno-compound-token-split-by-macro
^^ Maybe add a comment here or in the commit message that this occurs
with perl v5.34.
Together with the links Nathan pointing to the perl BTS?
> +endif
> +
> $(OUTPUT)test-libperl.bin:
> $(BUILD) $(FLAGS_PERL_EMBED)
>
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 96ad944ca6a885cd..5b5ba475a5c00c0f 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -790,6 +790,9 @@ else
> LDFLAGS += $(PERL_EMBED_LDFLAGS)
> EXTLIBS += $(PERL_EMBED_LIBADD)
> CFLAGS += -DHAVE_LIBPERL_SUPPORT
> + ifeq ($(CC_NO_CLANG), 0)
> + CFLAGS += -Wno-compound-token-split-by-macro
> + endif
> $(call detected,CONFIG_LIBPERL)
> endif
> endif
( I am here on Linux v5.17 + kbuild-gnu11-v5.18 and kbuild-v5.18 fixes. )
( My goal was to build perf with LLVM/Clang v13/v14 and -std=gnu11. )
( Debian/unstable switched to perl v5.34 in February. )
( That is the background I hit these issues. )
Link: https://marc.info/?t=164646683300002&r=1&w=2
Link: https://marc.info/?t=164689324800001&r=1&w=2
Unsure what your base is.
When I recall correctly we have CLANG_FLAGS exported in the top-level Makefile?
Oh no, now it is here.
$ git grep CLANG_FLAGS scripts/Makefile.clang
scripts/Makefile.clang:CLANG_FLAGS += --target=$(CLANG_TARGET_FLAGS)
scripts/Makefile.clang:CLANG_FLAGS += --target=$(notdir
$(CROSS_COMPILE:%-=%))
scripts/Makefile.clang:CLANG_FLAGS += -fno-integrated-as
scripts/Makefile.clang:CLANG_FLAGS +=
--prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
scripts/Makefile.clang:CLANG_FLAGS += -fintegrated-as
scripts/Makefile.clang:CLANG_FLAGS += -Werror=unknown-warning-option
scripts/Makefile.clang:CLANG_FLAGS += -Werror=ignored-optimization-argument
scripts/Makefile.clang:KBUILD_CFLAGS += $(CLANG_FLAGS)
scripts/Makefile.clang:KBUILD_AFLAGS += $(CLANG_FLAGS)
scripts/Makefile.clang:export CLANG_FLAGS
So, are all these CLANG_FLAGS "inherited" to tools/ build ECO systems?
I think no - it has its own rules :-(.
So, we should include scripts/Makefile.clang or enhance
tools/scripts/Makefile.include where LLVM/Clang stuff is defined (and
can be overridden).
Jiri pointed out he needed two options (when I recall correctly):
[1] -Wno-unused-command-line-argument
Example:
tools/testing/selftests/bpf/Makefile-# Silence some warnings when
compiled with clang
tools/testing/selftests/bpf/Makefile-ifneq ($(LLVM),)
tools/testing/selftests/bpf/Makefile:CFLAGS += -Wno-unused-command-line-argument
tools/testing/selftests/bpf/Makefile-endif
^^ Here you see it is checked for LLVM.
Again, put that stuff in one single place - tools/scripts/Makefile.include
[2] -Wno-ignored-optimization-argument
Both [1] and [2] were not needed in my case.
[1] is always good to have - not only for perf.
AFAICS, you wanted to consolidate PERL_EMBED_XXX and FLAGS_PERL_EMBED
at a single place.
1. tools/build/feature/Makefile
2. tools/perf/Makefile.config
Can not say where we all need perl check/requirement - maybe put that
stuff to the place where we define/check for toolchain/compiler (see
above)?
Again, all these issues happen because of -Werror is strictly set.
I put my patches around that area (see 2nd patch) - without checking
if the LLVM/Clang toolchain is used or not.
Both patches are attached.
$ git show -1 for-5.17/tools-feature_detect_libperl-clang-dileks-v3
commit 72ba634c9b39547197900d8b64ff36134af3ed08
(for-5.17/tools-feature_detect_libperl-clang-dileks-v3)
Author: Sedat Dilek <sedat.dilek@...il.com>
Date: Sun Mar 13 12:37:06 2022 +0100
tools: feature-detect: libperl: Sync PERL_EMBED_CCOPTS with perf
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 1480910c792e..fa6be127f483 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -217,7 +217,8 @@ strip-libs = $(filter-out -l%,$(1))
PERL_EMBED_LDOPTS = $(shell perl -MExtUtils::Embed -e ldopts 2>/dev/null)
PERL_EMBED_LDFLAGS = $(call strip-libs,$(PERL_EMBED_LDOPTS))
PERL_EMBED_LIBADD = $(call grep-libs,$(PERL_EMBED_LDOPTS))
-PERL_EMBED_CCOPTS = `perl -MExtUtils::Embed -e ccopts 2>/dev/null`
+PERL_EMBED_CCOPTS = $(perl -MExtUtils::Embed -e ccopts 2>/dev/null)
+PERL_EMBED_CCOPTS := $(filter-out -ffat-lto-objects, $(PERL_EMBED_CCOPTS))
FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
$(OUTPUT)test-libperl.bin:
$ git show -1 for-5.17/perf-libperl_support-clang-dileks
commit 316a1917ec05772ab0f99dad534fabdd74547865
(for-5.17/perf-libperl_support-clang-dileks)
Author: Sedat Dilek <sedat.dilek@...il.com>
Date: Sun Mar 6 11:29:50 2022 +0100
perf: Fix libperl support with clang and perl v5.34
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 1480910c792e..ef9b37c5c652 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -81,7 +81,7 @@ PKG_CONFIG ?= $(CROSS_COMPILE)pkg-config
all: $(FILES)
-__BUILD = $(CC) $(CFLAGS) -MD -Wall -Werror -o $@ $(patsubst
%.bin,%.c,$(@F)) $(LDFLAGS)
+__BUILD = $(CC) $(CFLAGS) -MD -Wall -Werror
-Wno-compound-token-split-by-macro -o $@ $(patsubst %.bin,%.c,$(@F))
$(LDFLAGS)
BUILD = $(__BUILD) > $(@:.bin=.make.output) 2>&1
__BUILDXX = $(CXX) $(CXXFLAGS) -MD -Wall -Werror -o $@ $(patsubst
%.bin,%.cpp,$(@F)) $(LDFLAGS)
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 96ad944ca6a8..acc987daf56e 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -213,7 +213,7 @@ endif
# Treat warnings as errors unless directed not to
ifneq ($(WERROR),0)
- CORE_CFLAGS += -Werror
+ CORE_CFLAGS += -Werror -Wno-compound-token-split-by-macro
CXXFLAGS += -Werror
HOSTCFLAGS += -Werror
endif
With both patches I am able to build perf-5.17 with LLVM/Clang v14 and
-std=gnu11 and perl v5.34.
It's good when Jiri tests on his system.
I can test your patch with my next kernel-build, but can not promise.
Do you have it somewhere in a Git repo/branch or a (LORE) link for
easy application?
Did not check if b4 tool is able to extract it as patch out of your
original email.
- Sedat -
View attachment "0001-tools-feature-detect-libperl-Sync-PERL_EMBED_CCOPTS-.patch" of type "text/x-patch" (1077 bytes)
View attachment "0001-perf-Fix-libperl-support-with-clang-and-perl-v5.34.patch" of type "text/x-patch" (1405 bytes)
Powered by blists - more mailing lists