| Message ID | 20190305001221.31343-1-ndesaulniers@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] x86/boot: clean up headers | expand |
On Mon, Mar 4, 2019 at 4:12 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > The inclusion of <linux/kernel.h> was causing issue as the definition of > __arch_hweight64 from arch/x86/include/asm/arch_hweight.h eventually gets > included. The definition is problematic when compiled with -m16 (all code > in arch/x86/boot/ is) as the "D" inline assembly constraint is rejected > by both compilers when passed an argument of type long long (regardless > of signedness, anything smaller is fine). > > Because GCC performs inlining before semantic analysis, and > __arch_hweight64 is dead in this translation unit, GCC does not report > any issues at compile time. Clang does the semantic analysis in the > front end, before inlining (run in the middle) can determine the code is > dead. I consider this another case of PR33587, which I think we can do > more work to solve. > > It turns out that arch/x86/boot/string.c doesn't actually need > linux/kernel.h, simply linux/limits.h and linux/compiler.h. Include them, > and sort the headers alphabetically. Well, should have cut that out of the commit message too in V3...sigh. Boris assuming everyone's ok with this patch; can you just drop this sentence from the commit message should you apply the patch (or shall I send a v4)? > > Link: https://bugs.llvm.org/show_bug.cgi?id=33587 > Link: https://github.com/ClangBuiltLinux/linux/issues/347 > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > Tested-by: Nathan Chancellor <natechancellor@gmail.com> > Suggested-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Changes V2 -> V3: > * keep linux/types.h > > Changes V1 -> V2: > * Add Reviewed, Tested, Suggested tags. > * Drop linux/types.h; it's included in linux/limits.h. > > arch/x86/boot/string.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c > index 315a67b8896b..90154df8f125 100644 > --- a/arch/x86/boot/string.c > +++ b/arch/x86/boot/string.c > @@ -13,8 +13,9 @@ > */ > > #include <linux/types.h> > -#include <linux/kernel.h> > +#include <linux/compiler.h> > #include <linux/errno.h> > +#include <linux/limits.h> > #include <asm/asm.h> > #include "ctype.h" > #include "string.h" > -- > 2.21.0.352.gf09ad66450-goog > -- Thanks, ~Nick Desaulniers
On Mon, Mar 04, 2019 at 04:15:49PM -0800, Nick Desaulniers wrote: > Well, should have cut that out of the commit message too in V3...sigh. > Boris assuming everyone's ok with this patch; can you just drop this > sentence from the commit message should you apply the patch (or shall > I send a v4)? I'll take a look once the merge window is over and all the dust settles. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c index 315a67b8896b..90154df8f125 100644 --- a/arch/x86/boot/string.c +++ b/arch/x86/boot/string.c @@ -13,8 +13,9 @@ */ #include <linux/types.h> -#include <linux/kernel.h> +#include <linux/compiler.h> #include <linux/errno.h> +#include <linux/limits.h> #include <asm/asm.h> #include "ctype.h" #include "string.h"