> On Thu, 2024-02-08 at 16:05 +0100, Jose E. Marchesi wrote:
>> > On Thu, 2024-02-08 at 13:55 +0100, Jose E. Marchesi wrote:
>> > [...]
>> >
>> > > However, it would be good if some clang wizard could confirm what
>> > > impact, if any, #pragma unroll (aka #pragma clang loop unroll(enabled))
>> > > has over -O2, before ditching these pragmas from the selftests.
>> >
>> > I compiled sefltests both with and without this patch,
>> > there are no differences in disassembly of generated BPF object files.
>> > (using current clang main).
>> >
>> > [...]
>>
>> Hmm, wouldn't that mean that the loops in profiler.inc.h never get
>> unrolled regardless of optimization level or pragma? (profiler2.c)
>>
>
> No, the generated code is different between profiler{1,2,3}, e.g.:
>
> $ llvm-objdump -d before/profiler1.bpf.o | wc -l
> 5356
> $ llvm-objdump -d before/profiler2.bpf.o | wc -l
> 2329
> $ llvm-objdump -d before/profiler3.bpf.o | wc -l
> 1915
>
> What I meant, is that generated code for before/profiler1.bpf.o
> and after/profiler1.bpf.o is identical, etc.
Right. But profiler2.c before and after the patch do:
--- Before:
profiler2.c:
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2020 Facebook */
#define barrier_var(var) /**/
/* undef #define UNROLL */
#define INLINE /**/
#include "profiler.inc.h"
profiler.inc.h:
#ifdef UNROLL
#pragma unroll
#endif
for (WHATEVER) {
[...]
}
--- After:
profiler2.c:
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2020 Facebook */
#define barrier_var(var) /**/
#define NO_UNROLL
#define INLINE /**/
#include "profiler.inc.h"
profiler.inc.h:
#ifdef NO_UNROLL
#pragma clang loop unroll(disable)
#endif
for (WHATEVER) {
[...]
}
---
If the compiler generates assembly code the same code for profile2.c for
before and after, that means that the loop does _not_ get unrolled when
profiler.inc.h is built with -O2 but without #pragma unroll.
But what if #pragma unroll is used? If it unrolls then, that would mean
that the pragma does something more than -funroll-loops/-O2.
Sorry if I am not making sense. Stuff like this confuses me to no end
;)
