[klibc] [PATCH v2 1/5] Kbuild: add support for clang builds

[Ben Hutchings]

On Fri, 2020-03-27 at 15:29 -0700, Bill Wendling wrote:
> From: Michael Davidson <md at google.com>
> 
> Add cc-name to klibc/scripts/Kbuild.include.
[...]
> --- a/scripts/Kbuild.klibc
> +++ b/scripts/Kbuild.klibc
> @@ -107,8 +107,15 @@ KLIBCOBJCOPY     := $(OBJCOPY)
>  KLIBCOBJDUMP     := $(OBJDUMP)
>  
>  # klibc include paths
> -KLIBCCPPFLAGS    := -nostdinc -iwithprefix include \
> -		    -I$(KLIBCINC)/arch/$(KLIBCARCHDIR)	\
> +ifeq ($(cc-name),clang)
> +# clang can't find the compiler-specific header <stdarg.h> if -nostdinc
> +# is specified so we have to omit it along with the -iwithprefix option.
> +# See b/35394554.

This seems to be an internal bug number.  It doesn't belong here.

> +KLIBCCPPFLAGS    :=
> +else
> +KLIBCCPPFLAGS    := -nostdinc -iwithprefix include
> +endif

It might make more sense to include these options only if cc-name is
"gcc".  (Doesn't make any difference now, but if we ever add a third
compiler then it seems like it ought to work without this.)

> +KLIBCCPPFLAGS    += -I$(KLIBCINC)/arch/$(KLIBCARCHDIR)	\
>                      -I$(KLIBCINC)/bits$(KLIBCBITSIZE)	\
>  		    -I$(KLIBCOBJ)/../include		\
>                      -I$(KLIBCINC)
> @@ -128,7 +135,13 @@ KLIBCCFLAGS      += $(KLIBCCPPFLAGS) $(KLIBCREQFLAGS) $(KLIBCARCHREQFLAGS)  \
>  KLIBCAFLAGS      += -D__ASSEMBLY__ -Wa,--noexecstack $(KLIBCCFLAGS)
>  KLIBCSTRIPFLAGS  += --strip-all -R .comment -R .note
>  
> +# KLIBCLIBGCC_DEF appears to be unnecessary since klibc never uses -nostdlib
> +# when linking binaries, which means that any compiler specific libraries
> +# should be available automatically, no need to speciy an explicit path.
> +# Don't attempt to set it if we are using clang.
> +ifneq ($(cc-name),clang)
>  KLIBCLIBGCC_DEF  := $(shell $(KLIBCCC) $(KLIBCCFLAGS) --print-libgcc)
> +endif

Seems like this should also check that cc-name is "gcc", then.

>  KLIBCLIBGCC	 ?= $(KLIBCLIBGCC_DEF)
>  KLIBCCRT0        := $(KLIBCOBJ)/arch/$(KLIBCARCHDIR)/crt0.o
>  KLIBCLIBC        := $(KLIBCOBJ)/libc.a
> diff --git a/usr/klibc/arch/x86_64/MCONFIG b/usr/klibc/arch/x86_64/MCONFIG
> index c5f2fa265a26..77e2ad279884 100644
> --- a/usr/klibc/arch/x86_64/MCONFIG
> +++ b/usr/klibc/arch/x86_64/MCONFIG
> @@ -15,13 +15,19 @@
>  # debugging using gdb.
>  #
>  KLIBCARCHREQFLAGS = -m64
> -KLIBCOPTFLAGS	+= -Os -fomit-frame-pointer -mno-sse \
> -		-falign-functions=1 -falign-jumps=1 -falign-loops=1
> +KLIBCOPTFLAGS	+= -Os -fomit-frame-pointer -mno-sse
>  ifeq ($(DEBUG),y)
>  KLIBCOPTFLAGS     += -g
>  else
>  KLIBCOPTFLAGS     += -fno-asynchronous-unwind-tables
>  endif
> +
> +ifneq ($(cc-name),clang)
> +# These options are not supported by clang but only result in a warning
> +# so we can't use a cc-option test to filter them.
> +KLIBCOPTFLAGS     += -falign-functions=1 -falign-jumps=1 -falign-loops=1
> +endif

Same here.  It would be more readable to write:

ifeq ($(cc-name),gcc)
KLIBCOPTFLAGS += gcc-specific-optimisations
endif
ifeq ($(cc-name),clang)
KLIBCOPTFLAGS += clang-specific-optimisations
endif

And this shouldn't need a comment.

Ben.

>  KLIBCBITSIZE      = 64
>  KLIBCLDFLAGS      = -m elf_x86_64



-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.zytor.com/archives/klibc/attachments/20200328/7c5abfaa/attachment.sig>