[llvmlinux] [PATCH] sched/fair: disable clang -Wconstant-logical-operand

Lukas Bulwahn lukas.bulwahn at gmail.com
Mon Aug 27 19:39:46 UTC 2018

On Mon, 27 Aug 2018, Peter Zijlstra wrote:

> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?
> A: Top-posting.
> Q: What is the most annoying thing in e-mail?
> On Mon, Aug 27, 2018 at 10:18:09AM -0700, Nick Desaulniers wrote:
> > Please file a bug.  There's more issues with clang in this regard:
> > https://bugs.llvm.org/show_bug.cgi?id=38571 (there should be more
> > warnings in this code).  IMO, we should fix the code (if under all
> > configs the expression contains constants), not disable the warning.
> It's just one config that produces the warning. IMO the warning is
> pretty useless.
> That said; I never got a response on:
>   https://lkml.kernel.org/r/20180420165139.GP4064@hirez.programming.kicks-ass.net
> which asked if:
>   #define sched_feat(x) !!(sysctl_sched_features & (1UL << __SCHED_FEAT_##x))
> would work, since that casts to a bool.

Peter, we did some further investigation since that last proposal above 
and considered three options:

Option 1: Modify the code, such that the compiler does not warn about it. 
IMO that would obfuscate the code just to make the compiler not warn about 
that (in your words) 'dodgy compiler warning'.

Option 2: Deactivate the compiler warning globally, as you considered the 
warning dodgy anyway.

Option 3: Deactivate the compiler warning locally, as there are cases 
where the compiler warning does point out mistakes, but in fair.c, it is 
simply a false positive.

You have seen that we proposed first a solution for option 1, but we 
thought it is counterproductive to modify readable and sane code, just to 
make the compiler not warn about it. So, we dropped option 1.

To decide on option 2 or 3, we searched for bugs and bug fixes in the 
kernel history that the warning would have indicated. We found about ten 
such cases where the warning would have been useful. Philipp can point to 
the specific cases we found.

So, we decided for option 3 and hence, this patch.


> _______________________________________________
> LLVMLinux mailing list
> LLVMLinux at lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/llvmlinux

More information about the LLVMLinux mailing list