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

[Artem Mygaiev <Artem_Mygaiev@xxxxxxxx>]

Hello Julien

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:

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.

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