| Message ID | 20190418093902.41835-1-abrodkin@synopsys.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | linux-tools/perf: Respect TARGET_CFLAGS | expand |
Hello, On Thu, 18 Apr 2019 12:39:02 +0300 Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > perf uses its own build-system and doesn't respect externally set flags. > Let's force it to use target flags as perf utility is nothing but > an ordinary user-space application. > > This among other things is required to build perf for ARC700 for which > we pass "-matomic" via TARGET_ABI, see [1] for more details. > > [1] https://patchwork.ozlabs.org/patch/1087471/ > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > --- > package/linux-tools/linux-tool-perf.mk.in | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/package/linux-tools/linux-tool-perf.mk.in b/package/linux-tools/linux-tool-perf.mk.in > index 80e00c3c56..e7565773c4 100644 > --- a/package/linux-tools/linux-tool-perf.mk.in > +++ b/package/linux-tools/linux-tool-perf.mk.in > @@ -19,6 +19,7 @@ PERF_MAKE_FLAGS = \ > JOBS=$(PARALLEL_JOBS) \ > ARCH=$(PERF_ARCH) \ > DESTDIR=$(TARGET_DIR) \ > + EXTRA_CFLAGS="$(TARGET_CFLAGS)" \ The change is OK, but I believe that if -matomic is really a core/important CFLAGS on ARC, then we should ideally pass it through the toolchain wrapper, so that we are sure it is always passed. Arnout, what do you think? Best regards, Thomas
On 18/04/2019 11:43, Thomas Petazzoni wrote: > Hello, > > On Thu, 18 Apr 2019 12:39:02 +0300 > Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > >> perf uses its own build-system and doesn't respect externally set flags. >> Let's force it to use target flags as perf utility is nothing but >> an ordinary user-space application. >> >> This among other things is required to build perf for ARC700 for which >> we pass "-matomic" via TARGET_ABI, see [1] for more details. >> >> [1] https://patchwork.ozlabs.org/patch/1087471/ >> >> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> >> --- >> package/linux-tools/linux-tool-perf.mk.in | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/package/linux-tools/linux-tool-perf.mk.in b/package/linux-tools/linux-tool-perf.mk.in >> index 80e00c3c56..e7565773c4 100644 >> --- a/package/linux-tools/linux-tool-perf.mk.in >> +++ b/package/linux-tools/linux-tool-perf.mk.in >> @@ -19,6 +19,7 @@ PERF_MAKE_FLAGS = \ >> JOBS=$(PARALLEL_JOBS) \ >> ARCH=$(PERF_ARCH) \ >> DESTDIR=$(TARGET_DIR) \ >> + EXTRA_CFLAGS="$(TARGET_CFLAGS)" \ > > The change is OK, but I believe that if -matomic is really a > core/important CFLAGS on ARC, then we should ideally pass it through > the toolchain wrapper, so that we are sure it is always passed. > > Arnout, what do you think? +1. Thomas, last year we made a beginning of deciding what should go through the wrapper and what through CFLAGS, but I'm not sure if we ever got to a conclusion? Regards, Arnout
Hi Arnout, Thomas, > -----Original Message----- > From: Arnout Vandecappelle <arnout@mind.be> > Sent: Sunday, April 21, 2019 10:36 AM > To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>; Alexey Brodkin <abrodkin@synopsys.com> > Cc: buildroot@busybox.net; ARC Buildroot mailing list <arc-buildroot@synopsys.com> > Subject: Re: [Buildroot] [PATCH] linux-tools/perf: Respect TARGET_CFLAGS > > > > On 18/04/2019 11:43, Thomas Petazzoni wrote: > > Hello, > > > > On Thu, 18 Apr 2019 12:39:02 +0300 > > Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > > > >> perf uses its own build-system and doesn't respect externally set flags. > >> Let's force it to use target flags as perf utility is nothing but > >> an ordinary user-space application. > >> > >> This among other things is required to build perf for ARC700 for which > >> we pass "-matomic" via TARGET_ABI, see [1] for more details. > >> > >> [1] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__patchwork.ozlabs.org_patch_1087471_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7b > reS55ytWkhpk5R81I&m=eB4KQfsAGH4782y3faOnCUr3nb3xgbJBuMv3P9P6cxY&s=g7hFDq2PCWQQNFtfEGkCnU8PyqSm-NS- > FzrkT4QMA8c&e= > >> > >> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > >> --- > >> package/linux-tools/linux-tool-perf.mk.in | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/package/linux-tools/linux-tool-perf.mk.in b/package/linux-tools/linux-tool-perf.mk.in > >> index 80e00c3c56..e7565773c4 100644 > >> --- a/package/linux-tools/linux-tool-perf.mk.in > >> +++ b/package/linux-tools/linux-tool-perf.mk.in > >> @@ -19,6 +19,7 @@ PERF_MAKE_FLAGS = \ > >> JOBS=$(PARALLEL_JOBS) \ > >> ARCH=$(PERF_ARCH) \ > >> DESTDIR=$(TARGET_DIR) \ > >> + EXTRA_CFLAGS="$(TARGET_CFLAGS)" \ > > > > The change is OK, but I believe that if -matomic is really a > > core/important CFLAGS on ARC, then we should ideally pass it through > > the toolchain wrapper, so that we are sure it is always passed. > > > > Arnout, what do you think? > > +1. > > Thomas, last year we made a beginning of deciding what should go through the > wrapper and what through CFLAGS, but I'm not sure if we ever got to a conclusion? So are we going to wait for some infrastructure changes or shall we first fix the problem at hand with broken perf build to ARC700? -Alexey
On 26/04/2019 08:30, Alexey Brodkin wrote: > Hi Arnout, Thomas, > >> -----Original Message----- >> From: Arnout Vandecappelle <arnout@mind.be> >> Sent: Sunday, April 21, 2019 10:36 AM >> To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>; Alexey Brodkin <abrodkin@synopsys.com> >> Cc: buildroot@busybox.net; ARC Buildroot mailing list <arc-buildroot@synopsys.com> >> Subject: Re: [Buildroot] [PATCH] linux-tools/perf: Respect TARGET_CFLAGS >> >> >> >> On 18/04/2019 11:43, Thomas Petazzoni wrote: >>> Hello, >>> >>> On Thu, 18 Apr 2019 12:39:02 +0300 >>> Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: >>> >>>> perf uses its own build-system and doesn't respect externally set flags. >>>> Let's force it to use target flags as perf utility is nothing but >>>> an ordinary user-space application. >>>> >>>> This among other things is required to build perf for ARC700 for which >>>> we pass "-matomic" via TARGET_ABI, see [1] for more details. >>>> >>>> [1] https://urldefense.proofpoint.com/v2/url?u=https- >> 3A__patchwork.ozlabs.org_patch_1087471_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7b >> reS55ytWkhpk5R81I&m=eB4KQfsAGH4782y3faOnCUr3nb3xgbJBuMv3P9P6cxY&s=g7hFDq2PCWQQNFtfEGkCnU8PyqSm-NS- >> FzrkT4QMA8c&e= >>>> >>>> Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> >>>> --- >>>> package/linux-tools/linux-tool-perf.mk.in | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/package/linux-tools/linux-tool-perf.mk.in b/package/linux-tools/linux-tool-perf.mk.in >>>> index 80e00c3c56..e7565773c4 100644 >>>> --- a/package/linux-tools/linux-tool-perf.mk.in >>>> +++ b/package/linux-tools/linux-tool-perf.mk.in >>>> @@ -19,6 +19,7 @@ PERF_MAKE_FLAGS = \ >>>> JOBS=$(PARALLEL_JOBS) \ >>>> ARCH=$(PERF_ARCH) \ >>>> DESTDIR=$(TARGET_DIR) \ >>>> + EXTRA_CFLAGS="$(TARGET_CFLAGS)" \ >>> >>> The change is OK, but I believe that if -matomic is really a >>> core/important CFLAGS on ARC, then we should ideally pass it through >>> the toolchain wrapper, so that we are sure it is always passed. >>> >>> Arnout, what do you think? >> >> +1. >> >> Thomas, last year we made a beginning of deciding what should go through the >> wrapper and what through CFLAGS, but I'm not sure if we ever got to a conclusion? > > So are we going to wait for some infrastructure changes or > shall we first fix the problem at hand with broken perf build to ARC700? It's not really infrastructure changes. Rather, it is a better way to fix the issue. You just need to move the -matomic from TARGET_ABI to TOOLCHAIN_WRAPPER_OPTS. The only tricky part is where that should be done. I think arch/arch.mk.arc would be the right place. But that indeed requires an infrastructural change, because the TOOLCHAIN_WRAPPER_OPTS = ... in toolchain-wrapper.mk has to change into a +=. Regards, Arnout Regards, Arnout
Hi Yann, all, > -----Original Message----- > From: Yann E. MORIN <yann.morin.1998@gmail.com> On Behalf Of Yann E. MORIN > Sent: Saturday, April 27, 2019 8:56 PM > To: Arnout Vandecappelle <arnout@mind.be> > Cc: Alexey Brodkin <abrodkin@synopsys.com>; Thomas Petazzoni <thomas.petazzoni@bootlin.com>; > buildroot@busybox.net; ARC Buildroot mailing list <arc-buildroot@synopsys.com> > Subject: Re: [Buildroot] [PATCH] linux-tools/perf: Respect TARGET_CFLAGS > > Arnout, Alexey, All, > > On 2019-04-27 19:39 +0200, Arnout Vandecappelle spake thusly: > > On 26/04/2019 08:30, Alexey Brodkin wrote: > [--SNIP--] > > >>> The change is OK, but I believe that if -matomic is really a > > >>> core/important CFLAGS on ARC, then we should ideally pass it through > > >>> the toolchain wrapper, so that we are sure it is always passed. > > >>> Arnout, what do you think? > > >> +1. > > >> Thomas, last year we made a beginning of deciding what should go through the > > >> wrapper and what through CFLAGS, but I'm not sure if we ever got to a conclusion? > > > So are we going to wait for some infrastructure changes or > > > shall we first fix the problem at hand with broken perf build to ARC700? > > > > It's not really infrastructure changes. Rather, it is a better way to fix the > > issue. > > > > You just need to move the -matomic from TARGET_ABI to TOOLCHAIN_WRAPPER_OPTS. > > > > The only tricky part is where that should be done. I think arch/arch.mk.arc > > would be the right place. But that indeed requires an infrastructural change, > > because the TOOLCHAIN_WRAPPER_OPTS = ... in toolchain-wrapper.mk has to change > > into a +=. > > I'm not too fond of this, but what about the following: > > diff --git a/arch/arch.mk.arc b/arch/arch.mk.arc > new file mode 100644 > index 0000000000..83503841b6 > --- /dev/null > +++ b/arch/arch.mk.arc > @@ -0,0 +1,3 @@ > +ifeq ($(BR2_arcle),y) > +ARCH_TOOLCHAIN_WRAPPER_OPTS = -matomic > +endif > diff --git a/toolchain/toolchain-wrapper.mk > b/toolchain/toolchain-wrapper.mk > index ca66fa7ba4..8f872506ec 100644 > --- a/toolchain/toolchain-wrapper.mk > +++ b/toolchain/toolchain-wrapper.mk > @@ -17,6 +17,7 @@ TOOLCHAIN_WRAPPER_ARGS = > $($(PKG)_TOOLCHAIN_WRAPPER_ARGS) > TOOLCHAIN_WRAPPER_ARGS += -DBR_SYSROOT='"$(STAGING_SUBDIR)"' > > TOOLCHAIN_WRAPPER_OPTS = \ > + $(ARCH_TOOLCHAIN_WRAPPER_OPTS) \ > $(call qstrip,$(BR2_SSP_OPTION)) \ > $(call qstrip,$(BR2_TARGET_OPTIMIZATION)) That looks good to me with only one change: prefer generic "BR2_arc" instead of little-endian only "BR2_arcle". Care to send-out a patch? -Alexey
Alexey, All, On 2019-05-08 13:19 +0000, Alexey Brodkin spake thusly: > From: Yann E. MORIN <yann.morin.1998@gmail.com> On Behalf Of Yann E. MORIN [--SNIP--] > > I'm not too fond of this, but what about the following: > > > > diff --git a/arch/arch.mk.arc b/arch/arch.mk.arc > > new file mode 100644 > > index 0000000000..83503841b6 > > --- /dev/null > > +++ b/arch/arch.mk.arc > > @@ -0,0 +1,3 @@ > > +ifeq ($(BR2_arcle),y) > > +ARCH_TOOLCHAIN_WRAPPER_OPTS = -matomic > > +endif > > diff --git a/toolchain/toolchain-wrapper.mk > > b/toolchain/toolchain-wrapper.mk > > index ca66fa7ba4..8f872506ec 100644 > > --- a/toolchain/toolchain-wrapper.mk > > +++ b/toolchain/toolchain-wrapper.mk > > @@ -17,6 +17,7 @@ TOOLCHAIN_WRAPPER_ARGS = > > $($(PKG)_TOOLCHAIN_WRAPPER_ARGS) > > TOOLCHAIN_WRAPPER_ARGS += -DBR_SYSROOT='"$(STAGING_SUBDIR)"' > > > > TOOLCHAIN_WRAPPER_OPTS = \ > > + $(ARCH_TOOLCHAIN_WRAPPER_OPTS) \ > > $(call qstrip,$(BR2_SSP_OPTION)) \ > > $(call qstrip,$(BR2_TARGET_OPTIMIZATION)) > > That looks good to me with only one change: prefer generic "BR2_arc" > instead of little-endian only "BR2_arcle". Yes, of course, it was just a quicky to demo what I was thinking... > Care to send-out a patch? Aw... But *you* are the arc guy! ;-) OK, will do. Regards, Yann E. MORIN.
Hi Yann, > -----Original Message----- > From: Yann E. MORIN <yann.morin.1998@free.fr> > Sent: Wednesday, May 8, 2019 5:31 PM > To: Alexey Brodkin <abrodkin@synopsys.com> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>; buildroot@busybox.net; ARC Buildroot mailing list > <arc-buildroot@synopsys.com>; Arnout Vandecappelle <arnout@mind.be> > Subject: Re: [arc-buildroot] [Buildroot] [PATCH] linux-tools/perf: Respect TARGET_CFLAGS > > Alexey, All, > > On 2019-05-08 13:19 +0000, Alexey Brodkin spake thusly: > > From: Yann E. MORIN <yann.morin.1998@gmail.com> On Behalf Of Yann E. MORIN > [--SNIP--] > > > I'm not too fond of this, but what about the following: > > > > > > diff --git a/arch/arch.mk.arc b/arch/arch.mk.arc > > > new file mode 100644 > > > index 0000000000..83503841b6 > > > --- /dev/null > > > +++ b/arch/arch.mk.arc > > > @@ -0,0 +1,3 @@ > > > +ifeq ($(BR2_arcle),y) > > > +ARCH_TOOLCHAIN_WRAPPER_OPTS = -matomic > > > +endif > > > diff --git a/toolchain/toolchain-wrapper.mk > > > b/toolchain/toolchain-wrapper.mk > > > index ca66fa7ba4..8f872506ec 100644 > > > --- a/toolchain/toolchain-wrapper.mk > > > +++ b/toolchain/toolchain-wrapper.mk > > > @@ -17,6 +17,7 @@ TOOLCHAIN_WRAPPER_ARGS = > > > $($(PKG)_TOOLCHAIN_WRAPPER_ARGS) > > > TOOLCHAIN_WRAPPER_ARGS += -DBR_SYSROOT='"$(STAGING_SUBDIR)"' > > > > > > TOOLCHAIN_WRAPPER_OPTS = \ > > > + $(ARCH_TOOLCHAIN_WRAPPER_OPTS) \ > > > $(call qstrip,$(BR2_SSP_OPTION)) \ > > > $(call qstrip,$(BR2_TARGET_OPTIMIZATION)) > > > > That looks good to me with only one change: prefer generic "BR2_arc" > > instead of little-endian only "BR2_arcle". > > Yes, of course, it was just a quicky to demo what I was thinking... > > > Care to send-out a patch? > > Aw... But *you* are the arc guy! ;-) > > OK, will do. I'm all for sending it myself but it will be like I'm stealing your nice idea - nothing more that that. Want me to do that instead? -Alexey
Alexey, All, On 2019-05-08 14:54 +0000, Alexey Brodkin spake thusly: [--SNIP--] > > > Care to send-out a patch? > > Aw... But *you* are the arc guy! ;-) > > OK, will do. > > I'm all for sending it myself but it will be like I'm stealing > your nice idea - nothing more that that. > Want me to do that instead? I've done the patches localy already, I just need a bit of time to test then and send the series. I shall not promise anything, but maybe in the evening/night. BTW, are all ARC cores impacted, or only the 7x0d ? Regards, Yann E. MORIN.
Alexey, All, On 2019-04-18 12:39 +0300, Alexey Brodkin spake thusly: > perf uses its own build-system and doesn't respect externally set flags. > Let's force it to use target flags as perf utility is nothing but > an ordinary user-space application. > > This among other things is required to build perf for ARC700 for which > we pass "-matomic" via TARGET_ABI, see [1] for more details. As was discussed in the thread, the proper option is now passed in via the wrapper and arch-specific CFLAGS: https://git.buildroot.org/buildroot/commit/?id=4281288d2d568d5ce748a0e66e0872f54fbe956c https://git.buildroot.org/buildroot/commit/?id=c568b4f37fa6d7f51e6d14d33d7eb75dfe26d7bf Thanks! Regards, Yann E. MORIN. > [1] https://patchwork.ozlabs.org/patch/1087471/ > > Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> > --- > package/linux-tools/linux-tool-perf.mk.in | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/package/linux-tools/linux-tool-perf.mk.in b/package/linux-tools/linux-tool-perf.mk.in > index 80e00c3c56..e7565773c4 100644 > --- a/package/linux-tools/linux-tool-perf.mk.in > +++ b/package/linux-tools/linux-tool-perf.mk.in > @@ -19,6 +19,7 @@ PERF_MAKE_FLAGS = \ > JOBS=$(PARALLEL_JOBS) \ > ARCH=$(PERF_ARCH) \ > DESTDIR=$(TARGET_DIR) \ > + EXTRA_CFLAGS="$(TARGET_CFLAGS)" \ > prefix=/usr \ > WERROR=0 \ > NO_LIBAUDIT=1 \ > -- > 2.16.2 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
diff --git a/package/linux-tools/linux-tool-perf.mk.in b/package/linux-tools/linux-tool-perf.mk.in index 80e00c3c56..e7565773c4 100644 --- a/package/linux-tools/linux-tool-perf.mk.in +++ b/package/linux-tools/linux-tool-perf.mk.in @@ -19,6 +19,7 @@ PERF_MAKE_FLAGS = \ JOBS=$(PARALLEL_JOBS) \ ARCH=$(PERF_ARCH) \ DESTDIR=$(TARGET_DIR) \ + EXTRA_CFLAGS="$(TARGET_CFLAGS)" \ prefix=/usr \ WERROR=0 \ NO_LIBAUDIT=1 \
perf uses its own build-system and doesn't respect externally set flags. Let's force it to use target flags as perf utility is nothing but an ordinary user-space application. This among other things is required to build perf for ARC700 for which we pass "-matomic" via TARGET_ABI, see [1] for more details. [1] https://patchwork.ozlabs.org/patch/1087471/ Signed-off-by: Alexey Brodkin <abrodkin@synopsys.com> --- package/linux-tools/linux-tool-perf.mk.in | 1 + 1 file changed, 1 insertion(+)