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

[Artem Mygaiev <Artem_Mygaiev@xxxxxxxx>]

Hi Julien

On Mon, 2019-11-18 at 06:18 +0000, Julien Grall wrote:
> 
> 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.

Unfortunately this is not the case - we need to specify 2 different
targets: 1st is for GNU tools which are still used when building with
armclang, for debug symbols dumping etc. (there is no replacement in
Arm toolstack for them, and this is not on safety critical path so we
are fine) and 2nd is for armclang itself because it has its own
triplets definition which is not consistent with those used by GNU
tools.

> 
> 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?

They are fixed and defined in Arm compiler doc, only 2 options exist
for Aarch32 and Aarch64 architectures: --target=aarch64-arm-none-eabi
or --target=arm-arm-none-eabi. See e.g. [2]

[2] 
https://developer.arm.com/docs/100748/0612/using-common-compiler-options/common-arm-compiler-toolchain-options

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