Re: [Xen-devel] [RFC 2/7] WIP: Compilation with ARM DS-6 compiler

[Julien Grall <julien@xxxxxxx>]

On 14/11/2019 14:12, Artem Mygaiev wrote:
> Hello Julien
Hi,

> On Thu, 2019-11-14 at 08:19 +0900, Julien Grall wrote:
> > On Thu, 14 Nov 2019, 02:15 Artem Mygaiev, <
> > Artem_Mygaiev@xxxxxxxx> wrote:
> > > Hi Jan,
> > >
> > > Sorry for delayed reply
> > >
> > > On Thu, 2019-11-07 at 08:31 +0100, Jan Beulich wrote:
> > > > On 06.11.2019 23:08, Artem Mygaiev wrote:
> > > > > On Wed, Nov 6, 2019 at 4:28 PM Jan Beulich <
> > > > > jbeulich@xxxxxxxx
> > > > > > wrote:
> > > > > > On 06.11.2019 10:19, Andrii Anisov wrote:
> > > > > > > --- a/Config.mk
> > > > > > > +++ b/Config.mk
> > > > > > > @@ -221,7 +221,9 @@ CFLAGS += -Wall -Wstrict-prototypes
> > > > > > >
> > > > > > >   $(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-
> > > after-
> > > > > > > statement)
> > > > > > >   $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-
> > > statement)
> > > > > > > +ifneq ($(armds),y)
> > > > > > >   $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-
> > > variable)
> > > > > > > +endif
> > > > > > >   $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
> > > > > > >
> > > > > > >   LDFLAGS += $(foreach i, $(EXTRA_LIB), -L$(i))
> > > > > > ... this would be necessary.
> > > > > I am very sorry, this patch does not have a proper description
> > > > > indeed.
> > > > >
> > > > > For this particular change - arm clang does not undestand
> > > > > -Wno-unused-but-set-variable
> > > > > option at all, that is why it is under !$(armds)
> > > > But avoiding to add options which the compiler doesn't understand
> > > > is the purpose of using cc-option-add, rather than blindly
> > > > adding
> > > > them to CFLAGS. What am I missing here?
> > > You are right, the script shall check the compiler option and avoid
> > > including it to CFLAGS. But armclang require '--target=...' to be
> > > specified in order to operate properly, and the proper fix shall be
> > > something like this (instead of 'ifneq' hack above):
> > >
> > > diff --git a/Config.mk b/Config.mk
> > > index 01487a7..abe8e44 100644
> > > --- a/Config.mk
> > > +++ b/Config.mk
> > > @@ -107,7 +107,7 @@ cc-option = $(shell if test -z "`echo
> > > 'void*p=1;' | \
> > >   # Usage: $(call cc-option-add CFLAGS,CC,-march=winchip-c6)
> > >   cc-option-add = $(eval $(call cc-option-add-
> > > closure,$(1),$(2),$(3)))
> > >   define cc-option-add-closure
> > > -    ifneq ($$(call cc-option,$$($(2)),$(3),n),n)
> > > +    ifneq ($$(call cc-option,$$($(2) $(1)),$(3),n),n)
> > >           $(1) += $(3)
> > >       endif
> > >   endef
> > >
> > > so that CFLAGS that are already defined and include '--target=...'
> > > option from config/arm*.mk are passed to compiler to make it happy.
> > > I
> > > am not sure if this breaks anything else so decided to go with ugly
> > > 'ifneq' hack and check how this can be solved later on.
> >
> > Why not including --target in CC variable as this was suggested for
> > clang?
> In case of armclang --target is not the same as CROSS_COMPILE, we would
> need to introduce an extra variable instead of CFLAGS and then pass it
> to the compiler in similar way -target passed to clang:
IHMO, --target (armds) and -target (clang) are exactly the same. You 
specify the targeted architecture to build. So I think we need a similar 
approach in the both case. Although, in clang there are a default one 
when not specified.
I agree that using CROSS_COMPILE is a bit of a stretch (even on clang). 
There was actually a lenghty discussion (see [1]) about the meaning of 
CROSS_COMPILE. Maybe we want to introduce a new variable (e.g. TARGET) 
that can be used to pass the triplet.
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index 3bf3462..4bcfc58 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -3,8 +3,8 @@ AR         = $(CROSS_COMPILE)ar
>   LD         = $(CROSS_COMPILE)ld
>   ifeq ($(clang),y)
>   ifeq ($(armds),y)
> -CC         = armclang
> -CXX        = armclang
> +CC         = armclang --target=$(ARMDS_TARGET)
> +CXX        = armclang --target=$(ARMDS_TARGET)
>   LD_LTO     = armlink --verbose --no_scanlib
>   LD         = armlink --verbose --no_scanlib
>   AS         = armasm
> diff --git a/config/arm32.mk b/config/arm32.mk
> index 5afed07..b4c8fb1 100644
> --- a/config/arm32.mk
> +++ b/config/arm32.mk
> @@ -4,10 +4,12 @@ CONFIG_ARM_$(XEN_OS) := y
>   
>   CONFIG_XEN_INSTALL_SUFFIX :=
>   
> +ARMDS_TARGET := arm-arm-none-eabi
> +
>   # Explicitly specifiy 32-bit ARM ISA since toolchain default can be
> -mthumb:
>   ifeq ($(armds),y)
>   # VE needed
> -CFLAGS += --target=arm-arm-none-eabi -march=armv7-a
> +CFLAGS += -march=armv7-a
>   else
>   CFLAGS += -marm # -march= -mcpu=
>   # Use only if calling $(LD) directly.
> diff --git a/config/arm64.mk b/config/arm64.mk
> index 46b203d..57a7335 100644
> --- a/config/arm64.mk
> +++ b/config/arm64.mk
> @@ -4,9 +4,11 @@ CONFIG_ARM_$(XEN_OS) := y
>   
>   CONFIG_XEN_INSTALL_SUFFIX :=
>   
> +ARMDS_TARGET := aarch64-arm-none-eabi
> +
>   ifeq ($(armds),y)
>   # VE needed
> -CFLAGS += --target=aarch64-arm-none-eabi -march=armv8.1-a+nofp+nosimd
> +CFLAGS += -march=armv8.1-a+nofp+nosimd
>   else
>   CFLAGS += #-marm -march= -mcpu= etc
>   # Use only if calling $(LD) directly.
>
> But personally, I really do not want to add more build variables and
> flags (would also drop the 'armds' if I find a way how). Instead, I'd
> prefer the idea of re-using known CFLAGS during the cc-option tests,
> but, as I wrote above, wasn't sure if this is a right/safe thing to do,
> so while working on it I just quickly hacked out the option causing
> issues limiting amount of changes.
The question here is whether the target is always fixed for arm64/arm32. 
Are the two triplets used the only existing for armds?
Cheers,

[1]  <20190327184531.30986-2-julien.grall@xxxxxxx>

--
Julien Grall

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