On Fri, Oct 20, 2023 at 7:19 AM Rasmus Villemoes
<rasmus.villem...@prevas.dk> wrote:
>
> On 20/10/2023 12.13, Richard Purdie wrote:
> > On Fri, 2023-10-20 at 12:03 +0200, Rasmus Villemoes wrote:
> >> On 20/10/2023 11.38, Richard Purdie wrote:
> >>> On Fri, 2023-10-20 at 10:10 +0200, Rasmus Villemoes wrote:
> >>>> On 19/10/2023 14.48, Richard Purdie wrote:
> >>
> >>>>> The fact this works suggests perf is ignoring TARGET_CFLAGS. Is there
> >>>>> anything in the perf build system where we should be passing in cflags
> >>>>> we really want used?
> >>>>
> >>>> Well, IIUC, the normal way TARGET_CFLAGS would take effect is via the
> >>>> 'export CFLAGS = "${TARGET_CFLAGS}'. But in the perf recipe, both
> >>>> do_compile and do_install start with
> >>>>
> >>>> # Linux kernel build system is expected to do the right thing
> >>>> unset CFLAGS
> >>>>
> >>>> And perf's build system does indeed add lots of flags of its own, at
> >>>> least for most TUs, but nowhere is any -f(macro/debug/file)-prefix being
> >>>> set.
> >>>>
> >>>> So I do think that TARGET_CC_ARCH is the best place for flags that we
> >>>> really want used. At least as an initial step, because this is known to
> >>>> work when using the security_flags.inc file. Maybe there's some cleaner
> >>>> way where we don't unset CFLAGS, but that could be a giant can of worms.
> >>>>
> >>>> And yes, a comment should be added. Is something like
> >>>>
> >>>> # Perf's build system adds its own optimization flags for most TUs,
> >>>> # overriding the flags included here. But for some, perf does not add
> >>>> # any -O option, so ensure the distro's chosen optimization gets used
> >>>> # for those. Since ${SELECTED_OPTIMIZATION} always includes
> >>>> # ${DEBUG_FLAGS} which in turn includes ${DEBUG_PREFIX_MAP}, this also
> >>>> # ensures perf is built with appropriate -f*-prefix-map options,
> >>>> # avoiding the 'buildpaths' QA warning.
> >>>>
> >>>> too verbose?
> >>>
> >>> If it were me I'd probably write:
> >>>
> >>> # The perf makefile has "unset CFLAGS" as "Linux kernel build system is
> >>> # expected to do the right thing".
> >>
> >> It's not the perf makefile, it's right there in our recipe itself. I
> >> don't think upstream perf is to blame.
> >
> > Sorry, I misunderstood. I thought this was the perf environment that
> > was doing this so in that case your comment does make much more sense
> > than mine.
>
> OK. Bruce, should I resend?
Yes, let's go for a resend with the comment added. If we can avoid even some of
the pain wondering about this again .. our future selves will thank us :)
>
> > Out of interest did you see what happens if you allow our CFLAGS in and
> > remove that unset?
>
> I don't think I've tried that, but I've tried a few other things that I
> thought would be cleaner but which then exploded, so in the end I
> decided that just lifting exactly what the poky distro does should be
> safest and have the highest likelyhood of being accepted.
This is not directed at Rasmus, but a general comment ..
For pretty much anything kernel, and perf has always strongly been in
that category, we've always unset everything and allowed the Makefiles
(and the nested mess of tests, includes and trickery) to set their own
flags. It has saved subtle runtime issues and this is one area that the
developers tend to know what they are doing.
When there's a chance to append, they've give us a variable to do so,
that's why we've always used EXTRA_CFLAGS, and that likely is the
right place to use here as well. But even then, we shouldn't pass ALL of
our flags in there, just what we think needs to be in addition to the
perf generated set of CFLAGS.
Bruce
>
> > I also wondered if EXTRA_CFLAGS in EXTRA_OEMAKE
> > might be an option?
>
> Based on quick reading of perf build documentation, yes, EXTRA_CFLAGS
> seems like a reasonable approach. What I'm a little worried about is the
> TUs where perf apparently does not by itself add all its cflags (-O6 and
> whatnot), so I'm not sure if they would be built with what is passed via
> EXTRA_CFLAGS.
>
> I will experiment sometime next week if I remember and nothing else
> preempts that plan.
>
> > I'm a bit worried that the current situation all feels a little
> > fragile.
>
> Agree. But the QA checks do help quite a lot here.
>
> Rasmus
>
--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#189524):
https://lists.openembedded.org/g/openembedded-core/message/189524
Mute This Topic: https://lists.openembedded.org/mt/102058904/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
