Re: [PATCH v2 05/18] perf test: Add deterministic workload

[James Clark]

On 03/06/2026 2:43 pm, Leo Yan wrote:
> On Wed, Jun 03, 2026 at 02:10:37PM +0100, James Clark wrote:
> > On 03/06/2026 12:27 pm, Leo Yan wrote:
> > > On Tue, Jun 02, 2026 at 03:26:47PM +0100, James Clark wrote:
> > >
> > > [...]
> > >
> > > > @@ -22,3 +23,4 @@ CFLAGS_brstack.o          = -g -O0 -fno-inline -U_FORTIFY_SOURCE
> > > >    CFLAGS_datasym.o          = -g -O0 -fno-inline -U_FORTIFY_SOURCE
> > > >    CFLAGS_traploop.o         = -g -O0 -fno-inline -U_FORTIFY_SOURCE
> > > >    CFLAGS_inlineloop.o       = -g -O2
> > > > +CFLAGS_deterministic.o    = -g -O0
> > >
> > > I have no strong opinion for using 'noinline' in source or using the
> > > global option '-fno-inline', just thought this is not easy to follow
> > > up if anyone (likely myself) will write a new workload for disabling
> > > inline. Could we have consistent style for this?
> > >
> > > For the patch itself:
> > >
> > > Reviewed-by: Leo Yan <leo.yan@xxxxxxx>
> >
> > Actually it's a fair question why some have -fno-inline and others do it in
> > the code, it could just be copied from when these were built by their shell
> > script tests. From a quick look I would say we can easily drop the
> > -fno-inline and do it in the code, and it's better to only noinline what's
> > needed rather than everything. But that's probably a change for another
> > time.
>
> Seems to me, `-fno-inline` is more reliable.
>
> I.e., in this patch deterministic() has no 'noinline' annotation, my
> understanding is the test expects it is not inlined. With `-fno-inline`
> flag, we don't need to worry anything is missed.
>
> Thanks,
> Leo

Ok I can replace the noinlines with -fno-inline then.