diff mbox series

[v2] x86/boot: clean up headers

Message ID 20190304175016.40538-1-ndesaulniers@google.com
State Superseded
Headers show
Series [v2] x86/boot: clean up headers | expand

Commit Message

Nick Desaulniers March 4, 2019, 5:50 p.m. UTC 
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.

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 V1 -> V2:
* Add Reviewed, Tested, Suggested tags.
* Drop linux/types.h; it's included in linux/limits.h.

My original intention was to unsort the headers (sorted in V1), but if
we drop the out of place linux/types.h, then we can insert the two more
specific headers in alphabetic order.

 arch/x86/boot/string.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.21.0.352.gf09ad66450-goog

Comments

Stephen Rothwell March 4, 2019, 10:51 p.m. UTC | #1 
Hi Nick,

On Mon,  4 Mar 2019 09:50:16 -0800 Nick Desaulniers <ndesaulniers@google.com> wrote:
>

> Changes V1 -> V2:

> * Add Reviewed, Tested, Suggested tags.

> * Drop linux/types.h; it's included in linux/limits.h.

> 

> My original intention was to unsort the headers (sorted in V1), but if

> we drop the out of place linux/types.h, then we can insert the two more

> specific headers in alphabetic order.


I don't understand why you care about the include file ordering.

Also, since this file uses things from linux/types.h, I would argue
that we need the explicit include of linux/types.h.  Just imagine if
someone at a later date reomves the linux/types.h include from
linux/limits.h (however unlikely that is).

Just do your bug fix.

-- 
Cheers,
Stephen Rothwell
diff mbox series

Patch

diff --git a/arch/x86/boot/string.c b/arch/x86/boot/string.c
index 315a67b8896b..597589cafb45 100644
--- a/arch/x86/boot/string.c
+++ b/arch/x86/boot/string.c
@@ -12,9 +12,9 @@ 
  * Very basic string functions
  */
 
-#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"