Re: [PATCH bpf-next v2] tools/bpf: turn on llvm alu32 attribute by default

From: Yonghong Song <yhs@fb.com>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@fb.com>, Kernel Team <Kernel-team@fb.com>,
	Jiong Wang <jiong.wang@netronome.com>
Subject: Re: [PATCH bpf-next v2] tools/bpf: turn on llvm alu32 attribute by default
Date: Wed, 23 Oct 2019 02:27:31 +0000	[thread overview]
Message-ID: <f666fdcd-9b02-ca47-509b-aaffb3cf7c09@fb.com> (raw)
In-Reply-To: <20191022192953.GB31343@pc-66.home>

On 10/22/19 12:29 PM, Daniel Borkmann wrote:
> On Mon, Oct 21, 2019 at 09:31:19PM -0700, Yonghong Song wrote:
>> llvm alu32 was introduced in llvm7:
>>    https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_rL325987&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=0VCVs-aItkaVLRJ9Jp7YeX0We2JPKzcY7p_83Hlkso4&s=M0ANvh80tDNZb5JzE5vj9IETkKD87L1jFkcRHShC6Rk&e=
>>    https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_rL325989&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=0VCVs-aItkaVLRJ9Jp7YeX0We2JPKzcY7p_83Hlkso4&s=LABlrq9E6tmCwrbU2bCQa_LwchCaL8Tk5GczMCO5Cvs&e=
>> Experiments showed that in general performance
>> is better with alu32 enabled:
>>    https://urldefense.proofpoint.com/v2/url?u=https-3A__lwn.net_Articles_775316_&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=0VCVs-aItkaVLRJ9Jp7YeX0We2JPKzcY7p_83Hlkso4&s=qSDIIkauxw9Y_8rYH0AlvB4nvu06reDuhsb0GxSpoBo&e=
>>
>> This patch turned on alu32 with no-flavor test_progs
>> which is tested most often. The flavor test at
>> no_alu32/test_progs can be used to test without
>> alu32 enabled. The Makefile check for whether
>> llvm supports '-mattr=+alu32 -mcpu=v3' is
>> removed as llvm7 should be available for recent
>> distributions and also latest llvm is preferred
>> to run bpf selftests.
>>
>> Note that jmp32 is checked by -mcpu=probe and
>> will be enabled if the host kernel supports it.
>>
>> Cc: Jiong Wang <jiong.wang@netronome.com>
>> Acked-by: Andrii Nakryiko <andriin@fb.com>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
> 
> Applied, thanks!
> 
> Would it make sense to include -mattr=+alu32 also into -mcpu=probe
> on LLVM side or is the rationale to not do it that this causes a
> penalty for various other, non-x86 archs when done by default
> (although they could opt-out at the same time via -mattr=-alu32)?

The current -mcpu=probe is mostly to provide whether particular
instruction(s) are supported by the kernel or not. This follows
traditional cpu concept. For -mattr=+alu32 case, instruction set
remains the same, but we need to probe verifier capability.

But I agree that for bpf probing verifier for alu32 support
is totally reasonable.

Jiong, could you help do an implementation in llvm side since
you are more familiar with what alu32 capability needs to be
checked for verifier? Thanks!