Re: [Minios-devel] [UNIKRAFT PATCH] Add support for clang compilation

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hi Alice,

thanks a lot for this work! It will (unfortunately) need a some rtework 
because we had recently changes on how build flags are propagated.
On 01.03.20 20:13, alicesuiu wrote:
> Remove some flags (-fno-tree-sra, -mno-fp-ret-in-387, -fno-reorder-blocks) to 
> add support for clang compilation.
> Add the CONFIG_COMPILER variable to define the compiler to be used (gcc or 
> clang). By default the variable is set to gcc.
> Use 'make COMPILER=clang' to compile with clang.
>
> Signed-off-by: Alice Suiu <alicesuiu17@xxxxxxxxx>
> ---
>   Makefile                    | 12 ++++++++++--
>   Makefile.uk                 |  6 +++---
>   arch/x86/x86_64/Makefile.uk |  6 +++---
>   3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 84adb16..5828891 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -481,11 +481,19 @@ ifneq ("$(origin CROSS_COMPILE)","undefined")
>   CONFIG_CROSS_COMPILE := $(CROSS_COMPILE:"%"=%)
>   endif
>   
> +# Use 'make COMPILER=clang' to compile with clang
> +ifeq ("$(origin COMPILER)", "command line")
> +  CONFIG_COMPILER := $(COMPILER)
> +endif
> +ifndef CONFIG_COMPILER
> +  CONFIG_COMPILER = gcc
> +endif
> +
>   include $(CONFIG_UK_BASE)/arch/$(UK_FAMILY)/Compiler.uk
>   
>   # Make variables (CC, etc...)
> -LD             := $(CONFIG_CROSS_COMPILE)gcc
> -CC             := $(CONFIG_CROSS_COMPILE)gcc
> +LD             := $(CONFIG_CROSS_COMPILE)$(CONFIG_COMPILER)
> +CC             := $(CONFIG_CROSS_COMPILE)$(CONFIG_COMPILER)
>   CPP           := $(CC)
>   CXX           := $(CPP)
>   GOC           := $(CONFIG_CROSS_COMPILE)gccgo-7
I guess Go can also compiled with clang? It is fine to add support 
later, I am asking out of curiosity.
> diff --git a/Makefile.uk b/Makefile.uk
> index 3f01a25..3890d62 100644
> --- a/Makefile.uk
> +++ b/Makefile.uk
> @@ -8,17 +8,17 @@ ASFLAGS     += -U __linux__ -U __FreeBSD__ -U __sun__ 
> -D__ASSEMBLY__
>   ASINCLUDES  += -nostdinc -nostdlib -I$(CONFIG_UK_BASE)/include
>   
>   CFLAGS      += -U __linux__ -U __FreeBSD__ -U __sun__
> -CFLAGS      += -fno-stack-protector -fno-omit-frame-pointer -fno-tree-sra
> +CFLAGS      += -fno-stack-protector -fno-omit-frame-pointer
>   CFLAGS      += -Wall -Wextra
>   CINCLUDES   += -nostdinc -nostdlib -I$(CONFIG_UK_BASE)/include
>   
>   CXXFLAGS    += -U __linux__ -U __FreeBSD__ -U __sun__
> -CXXFLAGS    += -fno-stack-protector -fno-omit-frame-pointer -fno-tree-sra
> +CXXFLAGS    += -fno-stack-protector -fno-omit-frame-pointer
>   CXXFLAGS    += -Wall -Wextra
>   CXXINCLUDES += -nostdinc -nostdlib -I$(CONFIG_UK_BASE)/include
>   
>   GOCFLAGS    += -U __linux__ -U __FreeBSD__ -U __sun__
> -GOCFLAGS    += -fno-stack-protector -fno-omit-frame-pointer -fno-tree-sra
> +GOCFLAGS    += -fno-stack-protector -fno-omit-frame-pointer
>   GOCFLAGS    += -fno-split-stack -Wall -Wextra
>   GOCINCLUDES += -nostdinc -nostdlib -I$(CONFIG_UK_BASE)/include
Please note that all language-independent flags moved to:
ARCHFLAGS, ISR_ARCHFLAGS, COMPFLAGS

>   
> diff --git a/arch/x86/x86_64/Makefile.uk b/arch/x86/x86_64/Makefile.uk
> index 8ec7d6b..285a4e9 100644
> --- a/arch/x86/x86_64/Makefile.uk
> +++ b/arch/x86/x86_64/Makefile.uk
> @@ -1,9 +1,9 @@
>   ASFLAGS  += -D__X86_64__
>   ASFLAGS  += -m64
>   CFLAGS   += -D__X86_64__
> -CFLAGS   += -m64 -mno-red-zone -fno-reorder-blocks 
> -fno-asynchronous-unwind-tables
> +CFLAGS   += -m64 -mno-red-zone -fno-asynchronous-unwind-tables
>   CXXFLAGS += -D__X86_64__
> -CXXFLAGS += -m64 -mno-red-zone -fno-reorder-blocks 
> -fno-asynchronous-unwind-tables
> +CXXFLAGS += -m64 -mno-red-zone -fno-asynchronous-unwind-tables
>   
Also those, while we still have the language-specific flags (ASFLAGS, 
CFLAGS, etc.). We separate them now.
>   CINCLUDES   += -I$(CONFIG_UK_BASE)/arch/x86/x86_64/include
>   ASINCLUDES  += -I$(CONFIG_UK_BASE)/arch/x86/x86_64/include
> @@ -11,7 +11,7 @@ CXXINCLUDES += -I$(CONFIG_UK_BASE)/arch/x86/x86_64/include
>   
>   # compiler flags to prevent use of extended (FP, SSE, AVX) registers.
>   # This is for files that contain trap/exception/interrupt handlers
> -NO_X86_EXTREGS_FLAGS := -mno-80387 -mno-fp-ret-in-387 -mno-mmx -mno-sse 
> -mno-avx
> +NO_X86_EXTREGS_FLAGS := -mno-80387 -mno-mmx -mno-sse -mno-avx
>   
>   ASFLAGS-$(CONFIG_MARCH_X86_64_GENERIC)     += -mtune=generic
>   CFLAGS-$(CONFIG_MARCH_X86_64_GENERIC)      += -mtune=generic
I think we should add and modify the compiler-related Makefile rules 
within `support/build/Makefile.rules`. We have there so far:
  gcc_version_ge    returns 'y' if current GCC version is >= 'y',
                    otherwise 'n'
  gcc_version_lt    returns 'y' if current GCC version is < 'y',
                    otherwise 'n'

These two are used to populate flags according to the current used GCC 
version. For instance:
  COMPFLAGS-$(call gcc_version_ge,7,0) += -flag_supported_with_gcc70

  error_if_gcc_version_lt  Will output an error that a newer GCC
                           is needed

I think I would add two functions that return 'y' and 'n' when clang and 
GCC is detected:
   have_clang
   have_gcc

Using those, it probably makes sense to update the three existing 
functions so that the first two return 'n' and the last one never fails 
when have_gcc returned 'n' (when clang is used). Additionally, I would 
add the same set of functions for clang:
   clang_version_ge
   clang_version_le
   error_if_clang_version_lt

They behave equivalently to the gcc functions so that nothing breaks 
when GCC is used instead of clang (have_clang).
With all of this you are able to re-coordinate the flags. All flags that 
fail with clang, you would keep for GCC with
   COMPFLAGS-$(call have_gcc) += -gcc_only_flag

With this you can also introduce clang specific flags and it can be 
applied to all flag variables, including the language specific ones and 
the ones concerning linking.
What do you think?

Thanks,

Simon

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel