Re: Change default cpu version from v1 to v3 in llvm20

[Yonghong Song <yonghong.song@linux.dev>]

On 9/3/24 11:35 AM, Daniel Borkmann wrote:
> On 9/3/24 4:50 PM, Yonghong Song wrote:
>> On 9/2/24 11:52 PM, Daniel Borkmann wrote:
>>> On 9/3/24 6:10 AM, Yonghong Song wrote:
>>>> Hi,
>>>>
>>>> Suggested by Alexei, I put a llvm20 diff to make cpu=v3 as the default
>>>> cpu version:
>>>>     https://github.com/llvm/llvm-project/pull/107008
>>>>
>>>> cpu=v3 has been introduced in llvm9 (2019 H2) and the kernel cpu=v3
>>>> support should be available around the same time although I
>>>> cannot remember the exact kernel version.
>>>>
>>>> There are two motivation to move cpu version default from v1 to v3.
>>>>
>>>> First, to resolve correct usage of code like
>>>>     (void)__sync_fetch_and_add(&ptr, value);
>>>> In cpu v1/v2, the above insn generates locked add insn, and
>>>> for cpu >= v3, the above insn generates atomic_fetch_add insn.
>>>> The atomic_fetch_add insn is the correct way for the eventual
>>>> insn for arm64. Otherwise, with locked add insn in arm64,
>>>> proper barrier will be missing and incorrect results may
>>>> be generated.
>>>>
>>>> Second, cpu=v3 should have better performance than cpu=v1
>>>> in most cases. In Meta, several years ago, we have conducted
>>>> performance evaluation to compare v1 and v3 for major bpf
>>>> programs running in our platform and we concluded v3 is
>>>> better than v1 in most cases and in other rare cases v1 and v3
>>>> have the same performance. So moving to v3 can help
>>>> performance too.
>>>>
>>>> If in rare cases, e.g. really old kernels, v1/v2 is the only
>>>> option, then users can set -mcpu=v1 explicitly.
>>>>
>>>> Please let us know if you still have some concerns in your
>>>> setup w.r.t. cpu v1->v3 transition.
>>>
>>> Sounds good to me! Is there a place somewhere in LLVM where this
>>> can be documented for the BPF backend (along with the various
>>> extensions), so that developers can find sth in the official LLVM
>>> docs if they search the web? I see that riscv and some other archs
>>> have documentation under [0] which seems to get deployed under [1].
>>
>> Thanks Daniel.
>>
>> Trying to have llvm doc for BPF backend has been discussed
>> before. IIRC, Fangrui Song suggested this when we tried to
>> add BPF reloc documents in Documentation/bpf/llvm_reloc.rst.
>> Eventually we added llvm_reloc.rst to kernel since this doc
>> is mostly interesting for kernel/bpf folks. We should add
>> an entry in bpf_devel_QA.rst to mention that default cpu
>> version change from v1 to v3.
>>
>> Not sure whether we should have the same doc in
>> llvm.org/docs/. Let me discuss with other folks on this.
>
> I was mostly thinking that not everyone might be looking into
> kernel docs (say, eBPF for Windows folks using LLVM), and at
> least on gcc docs/wiki you'll find information & quirks about
> gcc-bpf backend [2].
>
>   [2] https://gcc.gnu.org/wiki/BPFBackEnd

Thanks, Daniel. Eduard and I will look into this.

Yonghong

>
>>>   [0] https://github.com/llvm/llvm-project/tree/main/llvm/docs
>>>   [1] https://llvm.org/docs/RISCV/RISCVVectorExtension.html
>>>
>>> Thanks,
>>> Daniel
>