From: Anton Blanchard <hidden> Date: 2017-01-21 01:04:00
Hi,
We added:
BUILD_BUG_ON(!__builtin_constant_p(feature))
to cpu_has_feature() and mmu_has_feature() in order to catch usage
issues (such as cpu_has_feature(cpu_has_feature(X)). Unfortunately LLVM
isn't smart enough to resolve this, and it errors out.
I work around it in my clang/LLVM builds of the kernel, but I have just
discovered that it causes a lot of issues for the bcc (eBPF) trace tool
(which uses LLVM).
How should we work around this? Wrap the checks in !clang perhaps?
Anton
From: Anton Blanchard <hidden> Date: 2017-01-21 09:50:06
Hi,
We added:
BUILD_BUG_ON(!__builtin_constant_p(feature))
to cpu_has_feature() and mmu_has_feature() in order to catch usage
issues (such as cpu_has_feature(cpu_has_feature(X)). Unfortunately
LLVM isn't smart enough to resolve this, and it errors out.
I work around it in my clang/LLVM builds of the kernel, but I have
just discovered that it causes a lot of issues for the bcc (eBPF)
trace tool (which uses LLVM).
How should we work around this? Wrap the checks in !clang perhaps?
Looks like it's a weakness in LLVM with inlining:
#include <assert.h>
#if 1
static inline void foo(unsigned long x)
{
assert(__builtin_constant_p(x));
}
#else
#define foo(X) assert(__builtin_constant_p(X))
#endif
int main(void)
{
foo(1);
return 0;
}
And there is an old bug on it:
https://llvm.org/bugs/show_bug.cgi?id=4898
Anton
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2017-01-24 05:36:58
Anton Blanchard [off-list ref] writes:
We added:
BUILD_BUG_ON(!__builtin_constant_p(feature))
to cpu_has_feature() and mmu_has_feature() in order to catch usage
issues (such as cpu_has_feature(cpu_has_feature(X)). Unfortunately LLVM
isn't smart enough to resolve this, and it errors out.
I work around it in my clang/LLVM builds of the kernel, but I have just
discovered that it causes a lot of issues for the bcc (eBPF) trace tool
(which uses LLVM).
I didn't understand that part, but Aneesh explained to me that it's
because bcc pulls in the kernel-internal headers.
I guess as a quick fix we just have to #ifdef it, can you confirm this
works?
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2017-01-24 06:15:52
Anton Blanchard [off-list ref] writes:
quoted
We added:
BUILD_BUG_ON(!__builtin_constant_p(feature))
to cpu_has_feature() and mmu_has_feature() in order to catch usage
issues (such as cpu_has_feature(cpu_has_feature(X)). Unfortunately
LLVM isn't smart enough to resolve this, and it errors out.
I work around it in my clang/LLVM builds of the kernel, but I have
just discovered that it causes a lot of issues for the bcc (eBPF)
trace tool (which uses LLVM).
How should we work around this? Wrap the checks in !clang perhaps?
Looks like it's a weakness in LLVM with inlining:
#include <assert.h>
#if 1
static inline void foo(unsigned long x)
{
assert(__builtin_constant_p(x));
}
#else
#define foo(X) assert(__builtin_constant_p(X))
#endif
int main(void)
{
foo(1);
return 0;
}
OK. cpu_has_feature() is __always_inline:
static __always_inline bool cpu_has_feature(unsigned long feature)
{
int i;
BUILD_BUG_ON(!__builtin_constant_p(feature));
But I guess even that doesn't work?
Hmm, in fact it seems because we don't define
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING and CONFIG_OPTIMIZE_INLINING, we
get:
#define inline inline __attribute__((always_inline)) notrace
So in fact every inline function is marked always_inline all the time,
which seems dubious.
But still, it seems clang is ignoring always_inline.
cheers
On Tue, Jan 24, 2017 at 7:15 AM, Michael Ellerman [off-list ref] wrote:
Hmm, in fact it seems because we don't define
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING and CONFIG_OPTIMIZE_INLINING, we
get:
#define inline inline __attribute__((always_inline)) notrace
So in fact every inline function is marked always_inline all the time,
which seems dubious.
But still, it seems clang is ignoring always_inline.
I think the problem is that __builtin_constant_p() is usually false on
clang, at least for arguments
that are not constant pointers.
Arnd
From: Naveen N. Rao <hidden> Date: 2017-01-24 10:47:46
On 2017/01/24 04:36PM, Michael Ellerman wrote:
Anton Blanchard [off-list ref] writes:
quoted
We added:
BUILD_BUG_ON(!__builtin_constant_p(feature))
to cpu_has_feature() and mmu_has_feature() in order to catch usage
issues (such as cpu_has_feature(cpu_has_feature(X)). Unfortunately LLVM
isn't smart enough to resolve this, and it errors out.
I work around it in my clang/LLVM builds of the kernel, but I have just
discovered that it causes a lot of issues for the bcc (eBPF) trace tool
(which uses LLVM).
I didn't understand that part, but Aneesh explained to me that it's
because bcc pulls in the kernel-internal headers.
I guess as a quick fix we just have to #ifdef it, can you confirm this
works?
This works and solves the issue for me. In the absence of a better
approach, can you please push this for v4.10?
Tested-by: Naveen N. Rao <redacted>
Thanks,
Naveen
From: David Laight <hidden> Date: 2017-01-25 10:35:58
From: Michael Ellerman
Sent: 24 January 2017 06:16
Anton Blanchard [off-list ref] writes:
quoted
quoted
We added:
BUILD_BUG_ON(!__builtin_constant_p(feature))
to cpu_has_feature() and mmu_has_feature() in order to catch usage
issues (such as cpu_has_feature(cpu_has_feature(X)). Unfortunately
LLVM isn't smart enough to resolve this, and it errors out.
I work around it in my clang/LLVM builds of the kernel, but I have
just discovered that it causes a lot of issues for the bcc (eBPF)
trace tool (which uses LLVM).
How should we work around this? Wrap the checks in !clang perhaps?
Looks like it's a weakness in LLVM with inlining:
#include <assert.h>
#if 1
static inline void foo(unsigned long x)
Does making this 'const unsigned long x' help?
ISTR that making a slight difference to the instruction patterns
gcc would use.
#define inline inline __attribute__((always_inline)) notrace
=20
So in fact every inline function is marked always_inline all the time,
which seems dubious.
I've had to do that in the past to get gcc to inline some small leaf functi=
ons
that were only called once.
(That was for some embedded code where I don't actually want any function c=
alls
at all.)
To my mind 'inline' should mean 'always_inline' since you need to explicitl=
y
stop functions being inlined even when not marked 'inline'.
David
On Wed, Jan 25, 2017 at 11:35 AM, David Laight [off-list ref] wrote:
From: Michael Ellerman
quoted
#define inline inline __attribute__((always_inline)) notrace
So in fact every inline function is marked always_inline all the time,
which seems dubious.
I've had to do that in the past to get gcc to inline some small leaf functions
that were only called once.
(That was for some embedded code where I don't actually want any function calls
at all.)
To my mind 'inline' should mean 'always_inline' since you need to explicitly
stop functions being inlined even when not marked 'inline'.
On x86, this is configurable using OPTIMIZE_INLINING, but IIRC no
other architecture
supports it in mainline Linux. I have played a bit with enabling it on
ARM, which
showed a couple of build warnings and errors from the changed inlining, but
they were all fixable. I have not submitted that since I have not actually been
able to do extensive runtime testing on it.
It may turn out to be worthwhile for powerpc, where you have a much more limited
set of configurations you care about and you already do some regular testing.
Potentially this is more efficient than the current default, and any
function that
actually requires being forced inline can be annotated as __always_inline rather
than plain inline.
Arnd