[PATCH v5 0/5] clang improvements

[PATCH v5 0/5] clang improvements

[Roger Pau Monne]

Hello,

The following series re-enable the integrated clang assembler when the
features required in order to build Xen are available.

This series has been tested with clang 6, clang 3.5, gcc 6 and gcc 7
with indirect branch support.

Thanks, Roger.

Roger Pau Monne (5):
  build: do not hardcode AFLAGS for as-insn tests
  build: filter out command line assembler arguments
  x86/clang: restore integrated assembler usage with indirect thunks
  x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK
  build/clang: add a check whether the assembler supports .skip with
    labels

 Config.mk                       | 15 ++++++++-------
 xen/Rules.mk                    | 14 +++++++++-----
 xen/arch/x86/Makefile           |  6 +++---
 xen/arch/x86/Rules.mk           | 17 ++++++++++++++---
 xen/include/Makefile            |  2 +-
 xen/include/asm-x86/asm_defns.h |  3 +++
 6 files changed, 38 insertions(+), 19 deletions(-)

-- 
2.16.1


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

[PATCH v5 1/5] build: do not hardcode AFLAGS for as-insn tests

[Roger Pau Monne]

Hardcoding as-insn to use AFLAGS is not correct. For once the test is
performed using a C file with inline assembly, and secondly the flags
used can be passed by the caller together with the CC.

Fix as-insn-check to pass the flags given as parameter to the test.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 Config.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Config.mk b/Config.mk
index 51adc27d83..407472c3fc 100644
--- a/Config.mk
+++ b/Config.mk
@@ -157,9 +157,9 @@ ifndef XEN_HAS_CHECKPOLICY
 endif
 
 # as-insn: Check whether assembler supports an instruction.
-# Usage: cflags-y += $(call as-insn "insn",option-yes,option-no)
+# Usage: cflags-y += $(call as-insn CC FLAGS,"insn",option-yes,option-no)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
-                       | $(1) $(filter-out -M% %.d -include %/include/xen/config.h,$(AFLAGS)) \
+                       | $(filter-out -M% %.d -include %/include/xen/config.h,$(1)) \
                               -c -x c -o /dev/null - 2>&1),$(4),$(3))
 
 # as-insn-check: Add an option to compilation flags, but only if insn is
@@ -167,7 +167,7 @@ as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
 # Usage: $(call as-insn-check CFLAGS,CC,"nop",-DHAVE_GAS_NOP)
 as-insn-check = $(eval $(call as-insn-check-closure,$(1),$(2),$(3),$(4)))
 define as-insn-check-closure
-    ifeq ($$(call as-insn,$$($(2)),$(3),y,n),y)
+    ifeq ($$(call as-insn,$$($(2)) $$($(1)),$(3),y,n),y)
         $(1) += $(4)
     endif
 endef
-- 
2.16.1


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

Re: [PATCH v5 1/5] build: do not hardcode AFLAGS for as-insn tests

[Andrew Cooper]

On 20/02/18 14:10, Roger Pau Monne wrote:
> Hardcoding as-insn to use AFLAGS is not correct. For once the test is
> performed using a C file with inline assembly, and secondly the flags
> used can be passed by the caller together with the CC.
>
> Fix as-insn-check to pass the flags given as parameter to the test.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  Config.mk | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Config.mk b/Config.mk
> index 51adc27d83..407472c3fc 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -157,9 +157,9 @@ ifndef XEN_HAS_CHECKPOLICY
>  endif
>  
>  # as-insn: Check whether assembler supports an instruction.
> -# Usage: cflags-y += $(call as-insn "insn",option-yes,option-no)
> +# Usage: cflags-y += $(call as-insn CC FLAGS,"insn",option-yes,option-no)

Strictly speaking, this is $(call as-insn,CC FLAGS,"insn" ...

>  as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
> -                       | $(1) $(filter-out -M% %.d -include %/include/xen/config.h,$(AFLAGS)) \
> +                       | $(filter-out -M% %.d -include %/include/xen/config.h,$(1)) \
>                                -c -x c -o /dev/null - 2>&1),$(4),$(3))
>  
>  # as-insn-check: Add an option to compilation flags, but only if insn is
> @@ -167,7 +167,7 @@ as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
>  # Usage: $(call as-insn-check CFLAGS,CC,"nop",-DHAVE_GAS_NOP)

And similarly here.  I can fix this on commit.

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

>  as-insn-check = $(eval $(call as-insn-check-closure,$(1),$(2),$(3),$(4)))
>  define as-insn-check-closure
> -    ifeq ($$(call as-insn,$$($(2)),$(3),y,n),y)
> +    ifeq ($$(call as-insn,$$($(2)) $$($(1)),$(3),y,n),y)
>          $(1) += $(4)
>      endif
>  endef


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

[PATCH v5 2/5] build: filter out command line assembler arguments

[Roger Pau Monne]

If the assembler is not used. This happens when using cc -E or cc -S
for example. GCC will just ignore the -Wa,... when the assembler is
not called, but clang will complain loudly and fail.

Also enable passing -Wa,-I$(BASEDIR)/include to clang now that it's
safe to do so.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v4:
 - Also add the filter to the tree rules near the end of xen/Rules.mk.

Changes since v3:
 - Filter using '-Wa,%' instead of '-Wa%' so that -Wall is not
   matched.
 - Pass -Wa,-I$(BASEDIR)/include to clang also.
---
 xen/Rules.mk          | 6 +++---
 xen/arch/x86/Makefile | 6 +++---
 xen/arch/x86/Rules.mk | 5 +----
 xen/include/Makefile  | 2 +-
 4 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index da3c35ba36..2918019b92 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -208,13 +208,13 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): %.init.o: %.o Makefile
 	$(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
 
 %.i: %.c Makefile
-	$(CPP) $(CFLAGS) $< -o $@
+	$(CPP) $(filter-out -Wa$(comma)%,$(CFLAGS)) $< -o $@
 
 %.s: %.c Makefile
-	$(CC) $(CFLAGS) -S $< -o $@
+	$(CC) $(filter-out -Wa$(comma)%,$(CFLAGS)) -S $< -o $@
 
 # -std=gnu{89,99} gets confused by # as an end-of-line comment marker
 %.s: %.S Makefile
-	$(CPP) $(AFLAGS) $< -o $@
+	$(CPP) $(filter-out -Wa$(comma)%,$(AFLAGS)) $< -o $@
 
 -include $(DEPS_INCLUDE)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d903b7abb9..389096139c 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -215,15 +215,15 @@ efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o: $(BASEDIR)/arch/x86/ef
 efi/boot.init.o efi/runtime.o efi/compat.o efi/buildid.o: ;
 
 asm-offsets.s: $(TARGET_SUBARCH)/asm-offsets.c
-	$(CC) $(filter-out -flto,$(CFLAGS)) -S -o $@ $<
+	$(CC) $(filter-out -Wa$(comma)% -flto,$(CFLAGS)) -S -o $@ $<
 
 xen.lds: xen.lds.S
-	$(CC) -P -E -Ui386 $(AFLAGS) -o $@ $<
+	$(CC) -P -E -Ui386 $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
 	sed -e 's/xen\.lds\.o:/xen\.lds:/g' <.xen.lds.d >.xen.lds.d.new
 	mv -f .xen.lds.d.new .xen.lds.d
 
 efi.lds: xen.lds.S
-	$(CC) -P -E -Ui386 -DEFI $(AFLAGS) -o $@ $<
+	$(CC) -P -E -Ui386 -DEFI $(filter-out -Wa$(comma)%,$(AFLAGS)) -o $@ $<
 	sed -e 's/efi\.lds\.o:/efi\.lds:/g' <.$(@F).d >.$(@F).d.new
 	mv -f .$(@F).d.new .$(@F).d
 
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 56b2ea8356..1dc5c3785a 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -42,8 +42,5 @@ CFLAGS += -DCONFIG_INDIRECT_THUNK
 export CONFIG_INDIRECT_THUNK=y
 endif
 
-# Set up the assembler include path properly for older GCC toolchains.  Clang
-# objects to the agument being passed however.
-ifneq ($(clang),y)
+# Set up the assembler include path properly for older toolchains.
 CFLAGS += -Wa,-I$(BASEDIR)/include
-endif
diff --git a/xen/include/Makefile b/xen/include/Makefile
index 19066a33a0..69052ade24 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -65,7 +65,7 @@ compat/%.h: compat/%.i Makefile $(BASEDIR)/tools/compat-build-header.py
 	mv -f $@.new $@
 
 compat/%.i: compat/%.c Makefile
-	$(CPP) $(filter-out -M% %.d -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
+	$(CPP) $(filter-out -Wa$(comma)% -M% %.d -include %/include/xen/config.h,$(CFLAGS)) $(cppflags-y) -o $@ $<
 
 compat/%.c: public/%.h xlat.lst Makefile $(BASEDIR)/tools/compat-build-source.py
 	mkdir -p $(@D)
-- 
2.16.1


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

Re: [PATCH v5 2/5] build: filter out command line assembler arguments

[Andrew Cooper]

On 20/02/18 14:10, Roger Pau Monne wrote:
> If the assembler is not used. This happens when using cc -E or cc -S
> for example. GCC will just ignore the -Wa,... when the assembler is
> not called, but clang will complain loudly and fail.
>
> Also enable passing -Wa,-I$(BASEDIR)/include to clang now that it's
> safe to do so.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

[PATCH v5 3/5] x86/clang: restore integrated assembler usage with indirect thunks

[Roger Pau Monne]

If the required features are met by the integrated clang assembler
(support for .includes and propagation of .macro-s between asm()-s)
do not disable it.

Only disable the integrated assembler for assembly files, like it was
done prior to "x86: Support indirect thunks from assembly code".

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v4:
 - Do not use else ifeq on the same line to be compatible with make
   3.8.
 - Modify as-insn-check to allow adding flags if the test case fails.

Changes since v3:
 - Do not modify how the thunk is included, clang upstream (and 6) has
   been fixed to propagate .macro-s between asm()-s.

Changes since v1:
 - Use as-insn to check if the assembler supports .include.
 - Open code a check for whether the assembler forgets .macro-s
   between asm()-s.
---
 Config.mk             |  9 +++++----
 xen/Rules.mk          |  5 +++--
 xen/arch/x86/Rules.mk | 14 ++++++++++++++
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/Config.mk b/Config.mk
index 407472c3fc..8d6d984488 100644
--- a/Config.mk
+++ b/Config.mk
@@ -162,13 +162,14 @@ as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
                        | $(filter-out -M% %.d -include %/include/xen/config.h,$(1)) \
                               -c -x c -o /dev/null - 2>&1),$(4),$(3))
 
-# as-insn-check: Add an option to compilation flags, but only if insn is
-#                supported by assembler.
-# Usage: $(call as-insn-check CFLAGS,CC,"nop",-DHAVE_GAS_NOP)
-as-insn-check = $(eval $(call as-insn-check-closure,$(1),$(2),$(3),$(4)))
+# as-insn-check: Conditionally add an option to compilation flags
+# Usage: $(call as-insn-check CFLAGS,CC,"nop",-DHAVE_GAS_NOP,-DNO_GAS_NOP)
+as-insn-check = $(eval $(call as-insn-check-closure,$(1),$(2),$(3),$(4),$(5)))
 define as-insn-check-closure
     ifeq ($$(call as-insn,$$($(2)) $$($(1)),$(3),y,n),y)
         $(1) += $(4)
+    else
+        $(1) += $(5)
     endif
 endef
 
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 2918019b92..7adf757fb6 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -70,8 +70,9 @@ endif
 
 AFLAGS-y                += -D__ASSEMBLY__
 
-# Clang's built-in assembler can't handle embedded .include's
-CFLAGS-$(clang)         += -no-integrated-as
+# Older clang's built-in assembler doesn't understand .skip with labels:
+# https://bugs.llvm.org/show_bug.cgi?id=27369
+AFLAGS-$(clang)         += -no-integrated-as
 
 ALL_OBJS := $(ALL_OBJS-y)
 
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 1dc5c3785a..cee83d392e 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -44,3 +44,17 @@ endif
 
 # Set up the assembler include path properly for older toolchains.
 CFLAGS += -Wa,-I$(BASEDIR)/include
+
+ifeq ($(clang),y)
+    # Check whether clang asm()-s support .include.
+    $(call as-insn-check,CFLAGS,CC,".include \"asm/indirect_thunk_asm.h\"",, \
+                         -no-integrated-as)
+    # Check whether clang keeps .macro-s between asm()-s:
+    # https://bugs.llvm.org/show_bug.cgi?id=36110
+    ifeq ($(if $(shell echo 'void _(void) { asm volatile ( ".macro FOO\n.endm" ); \
+                                            asm volatile ( ".macro FOO\n.endm" ); }' \
+                       | $(CC) $(filter-out -M% %.d -include %/include/xen/config.h,$(CFLAGS)) \
+                               -c -x c -o /dev/null - 2>&1),n,y),y)
+        CFLAGS += -no-integrated-as
+    endif
+endif
-- 
2.16.1


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

Re: [PATCH v5 3/5] x86/clang: restore integrated assembler usage with indirect thunks

[Andrew Cooper]

On 20/02/18 14:10, Roger Pau Monne wrote:
> If the required features are met by the integrated clang assembler
> (support for .includes and propagation of .macro-s between asm()-s)
> do not disable it.
>
> Only disable the integrated assembler for assembly files, like it was
> done prior to "x86: Support indirect thunks from assembly code".
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> Changes since v4:
>  - Do not use else ifeq on the same line to be compatible with make
>    3.8.
>  - Modify as-insn-check to allow adding flags if the test case fails.
>
> Changes since v3:
>  - Do not modify how the thunk is included, clang upstream (and 6) has
>    been fixed to propagate .macro-s between asm()-s.
>
> Changes since v1:
>  - Use as-insn to check if the assembler supports .include.
>  - Open code a check for whether the assembler forgets .macro-s
>    between asm()-s.
> ---
>  Config.mk             |  9 +++++----
>  xen/Rules.mk          |  5 +++--
>  xen/arch/x86/Rules.mk | 14 ++++++++++++++
>  3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/Config.mk b/Config.mk
> index 407472c3fc..8d6d984488 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -162,13 +162,14 @@ as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
>                         | $(filter-out -M% %.d -include %/include/xen/config.h,$(1)) \
>                                -c -x c -o /dev/null - 2>&1),$(4),$(3))
>  
> -# as-insn-check: Add an option to compilation flags, but only if insn is
> -#                supported by assembler.
> -# Usage: $(call as-insn-check CFLAGS,CC,"nop",-DHAVE_GAS_NOP)
> -as-insn-check = $(eval $(call as-insn-check-closure,$(1),$(2),$(3),$(4)))
> +# as-insn-check: Conditionally add an option to compilation flags
> +# Usage: $(call as-insn-check CFLAGS,CC,"nop",-DHAVE_GAS_NOP,-DNO_GAS_NOP)
> +as-insn-check = $(eval $(call as-insn-check-closure,$(1),$(2),$(3),$(4),$(5)))

It is a shame about the naming.  This is really as-insn-add, even before
your changes.  I'll prepare a separate patch to rename it, which should
ideally come ahead of this patch.

>  define as-insn-check-closure
>      ifeq ($$(call as-insn,$$($(2)) $$($(1)),$(3),y,n),y)
>          $(1) += $(4)
> +    else
> +        $(1) += $(5)
>      endif
>  endef
>  
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 2918019b92..7adf757fb6 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -70,8 +70,9 @@ endif
>  
>  AFLAGS-y                += -D__ASSEMBLY__
>  
> -# Clang's built-in assembler can't handle embedded .include's
> -CFLAGS-$(clang)         += -no-integrated-as
> +# Older clang's built-in assembler doesn't understand .skip with labels:
> +# https://bugs.llvm.org/show_bug.cgi?id=27369
> +AFLAGS-$(clang)         += -no-integrated-as

What age is "Older" ?  As soon as these build fixes are in, I will
refresh my automatic padding calculation series, which will add skip
with labels to C code as well.

>  
>  ALL_OBJS := $(ALL_OBJS-y)
>  
> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
> index 1dc5c3785a..cee83d392e 100644
> --- a/xen/arch/x86/Rules.mk
> +++ b/xen/arch/x86/Rules.mk
> @@ -44,3 +44,17 @@ endif
>  
>  # Set up the assembler include path properly for older toolchains.
>  CFLAGS += -Wa,-I$(BASEDIR)/include
> +
> +ifeq ($(clang),y)
> +    # Check whether clang asm()-s support .include.
> +    $(call as-insn-check,CFLAGS,CC,".include \"asm/indirect_thunk_asm.h\"",, \
> +                         -no-integrated-as)
> +    # Check whether clang keeps .macro-s between asm()-s:
> +    # https://bugs.llvm.org/show_bug.cgi?id=36110
> +    ifeq ($(if $(shell echo 'void _(void) { asm volatile ( ".macro FOO\n.endm" ); \
> +                                            asm volatile ( ".macro FOO\n.endm" ); }' \
> +                       | $(CC) $(filter-out -M% %.d -include %/include/xen/config.h,$(CFLAGS)) \
> +                               -c -x c -o /dev/null - 2>&1),n,y),y)
> +        CFLAGS += -no-integrated-as

This can end up adding -no-integrated-as twice, which would be more
obvious with as-insn-check renamed to as-insn-add.

I've got a plan to make this a lot cleaner.

~Andrew

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

Re: [PATCH v5 3/5] x86/clang: restore integrated assembler usage with indirect thunks

[Roger Pau Monné]

On Wed, Feb 21, 2018 at 04:56:14PM +0000, Andrew Cooper wrote:
> On 20/02/18 14:10, Roger Pau Monne wrote:
> > If the required features are met by the integrated clang assembler
> > (support for .includes and propagation of .macro-s between asm()-s)
> > do not disable it.
> >
> > Only disable the integrated assembler for assembly files, like it was
> > done prior to "x86: Support indirect thunks from assembly code".
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Changes since v4:
> >  - Do not use else ifeq on the same line to be compatible with make
> >    3.8.
> >  - Modify as-insn-check to allow adding flags if the test case fails.
> >
> > Changes since v3:
> >  - Do not modify how the thunk is included, clang upstream (and 6) has
> >    been fixed to propagate .macro-s between asm()-s.
> >
> > Changes since v1:
> >  - Use as-insn to check if the assembler supports .include.
> >  - Open code a check for whether the assembler forgets .macro-s
> >    between asm()-s.
> > ---
> >  Config.mk             |  9 +++++----
> >  xen/Rules.mk          |  5 +++--
> >  xen/arch/x86/Rules.mk | 14 ++++++++++++++
> >  3 files changed, 22 insertions(+), 6 deletions(-)
> >
> > diff --git a/Config.mk b/Config.mk
> > index 407472c3fc..8d6d984488 100644
> > --- a/Config.mk
> > +++ b/Config.mk
> > @@ -162,13 +162,14 @@ as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
> >                         | $(filter-out -M% %.d -include %/include/xen/config.h,$(1)) \
> >                                -c -x c -o /dev/null - 2>&1),$(4),$(3))
> >  
> > -# as-insn-check: Add an option to compilation flags, but only if insn is
> > -#                supported by assembler.
> > -# Usage: $(call as-insn-check CFLAGS,CC,"nop",-DHAVE_GAS_NOP)
> > -as-insn-check = $(eval $(call as-insn-check-closure,$(1),$(2),$(3),$(4)))
> > +# as-insn-check: Conditionally add an option to compilation flags
> > +# Usage: $(call as-insn-check CFLAGS,CC,"nop",-DHAVE_GAS_NOP,-DNO_GAS_NOP)
> > +as-insn-check = $(eval $(call as-insn-check-closure,$(1),$(2),$(3),$(4),$(5)))
> 
> It is a shame about the naming.  This is really as-insn-add, even before
> your changes.  I'll prepare a separate patch to rename it, which should
> ideally come ahead of this patch.

Renaming would be fine for me, although -check doesn't seem that bad
TBH.

> >  define as-insn-check-closure
> >      ifeq ($$(call as-insn,$$($(2)) $$($(1)),$(3),y,n),y)
> >          $(1) += $(4)
> > +    else
> > +        $(1) += $(5)
> >      endif
> >  endef
> >  
> > diff --git a/xen/Rules.mk b/xen/Rules.mk
> > index 2918019b92..7adf757fb6 100644
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -70,8 +70,9 @@ endif
> >  
> >  AFLAGS-y                += -D__ASSEMBLY__
> >  
> > -# Clang's built-in assembler can't handle embedded .include's
> > -CFLAGS-$(clang)         += -no-integrated-as
> > +# Older clang's built-in assembler doesn't understand .skip with labels:
> > +# https://bugs.llvm.org/show_bug.cgi?id=27369
> > +AFLAGS-$(clang)         += -no-integrated-as
> 
> What age is "Older" ?  As soon as these build fixes are in, I will
> refresh my automatic padding calculation series, which will add skip
> with labels to C code as well.

Hm, OK, then we might want to merge patch 5 with this one and move
patch 4 before this one. And use CFLAGS instead of AFLAGS here.

> >  
> >  ALL_OBJS := $(ALL_OBJS-y)
> >  
> > diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
> > index 1dc5c3785a..cee83d392e 100644
> > --- a/xen/arch/x86/Rules.mk
> > +++ b/xen/arch/x86/Rules.mk
> > @@ -44,3 +44,17 @@ endif
> >  
> >  # Set up the assembler include path properly for older toolchains.
> >  CFLAGS += -Wa,-I$(BASEDIR)/include
> > +
> > +ifeq ($(clang),y)
> > +    # Check whether clang asm()-s support .include.
> > +    $(call as-insn-check,CFLAGS,CC,".include \"asm/indirect_thunk_asm.h\"",, \
> > +                         -no-integrated-as)
> > +    # Check whether clang keeps .macro-s between asm()-s:
> > +    # https://bugs.llvm.org/show_bug.cgi?id=36110
> > +    ifeq ($(if $(shell echo 'void _(void) { asm volatile ( ".macro FOO\n.endm" ); \
> > +                                            asm volatile ( ".macro FOO\n.endm" ); }' \
> > +                       | $(CC) $(filter-out -M% %.d -include %/include/xen/config.h,$(CFLAGS)) \
> > +                               -c -x c -o /dev/null - 2>&1),n,y),y)
> > +        CFLAGS += -no-integrated-as
> 
> This can end up adding -no-integrated-as twice, which would be more
> obvious with as-insn-check renamed to as-insn-add.

-no-integrated-as will only be added once, if it's added in the
as-insn-check above this test will fail (because GNU as will be
used here), and thus the flag won't be added.

Thanks, Roger.

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

[PATCH v5 4/5] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK

[Roger Pau Monne]

When indirect_thunk_asm.h is instantiated directly into assembly files
CONFIG_INDIRECT_THUNK might not be defined, and thus using .if against
it is wrong.

Add a check to define CONFIG_INDIRECT_THUNK to 0 if not defined, so
that using .if CONFIG_INDIRECT_THUNK is always correct.

This suppresses the following clang error:

<instantiation>:8:9: error: expected absolute expression
    .if CONFIG_INDIRECT_THUNK == 1
        ^
<instantiation>:1:1: note: while in macro instantiation
INDIRECT_BRANCH call %rdx
^
entry.S:589:9: note: while in macro instantiation
        INDIRECT_CALL %rdx
        ^

Note that this is a preparatory patch in order to enable clang's
integrated assembler, the integrated assembler is not yet enabled for
assembly files.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v4:
 - Define CONFIG_INDIRECT_THUNK if not defined using an equation.
---
 xen/include/asm-x86/asm_defns.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index 6fc13d39d8..ebd2c88a1f 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -15,6 +15,9 @@
 #include <asm/alternative.h>
 
 #ifdef __ASSEMBLY__
+#ifndef CONFIG_INDIRECT_THUNK
+.equ CONFIG_INDIRECT_THUNK, 0
+#endif
 # include <asm/indirect_thunk_asm.h>
 #else
 asm ( "\t.equ CONFIG_INDIRECT_THUNK, "
-- 
2.16.1


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

Re: [PATCH v5 4/5] x86: fix indirect thunk usage of CONFIG_INDIRECT_THUNK

[Jan Beulich]

>>> On 20.02.18 at 15:10, <roger.pau@citrix.com> wrote:
> When indirect_thunk_asm.h is instantiated directly into assembly files
> CONFIG_INDIRECT_THUNK might not be defined, and thus using .if against
> it is wrong.
> 
> Add a check to define CONFIG_INDIRECT_THUNK to 0 if not defined, so
> that using .if CONFIG_INDIRECT_THUNK is always correct.
> 
> This suppresses the following clang error:
> 
> <instantiation>:8:9: error: expected absolute expression
>     .if CONFIG_INDIRECT_THUNK == 1
>         ^
> <instantiation>:1:1: note: while in macro instantiation
> INDIRECT_BRANCH call %rdx
> ^
> entry.S:589:9: note: while in macro instantiation
>         INDIRECT_CALL %rdx
>         ^
> 
> Note that this is a preparatory patch in order to enable clang's
> integrated assembler, the integrated assembler is not yet enabled for
> assembly files.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


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

[PATCH v5 5/5] build/clang: add a check whether the assembler supports .skip with labels

[Roger Pau Monne]

Or else disable the integrated assembler for assembly files. This is
relevant for older clang versions which integrated assembler don't
support .skip with labels.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 xen/Rules.mk | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 7adf757fb6..92102e36d9 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -72,7 +72,10 @@ AFLAGS-y                += -D__ASSEMBLY__
 
 # Older clang's built-in assembler doesn't understand .skip with labels:
 # https://bugs.llvm.org/show_bug.cgi?id=27369
-AFLAGS-$(clang)         += -no-integrated-as
+ifeq ($(clang),y)
+$(call as-insn-check,AFLAGS,CC,".L0:\n.L1:\n.skip (.L1 - .L0)",, \
+                     -no-integrated-as)
+endif
 
 ALL_OBJS := $(ALL_OBJS-y)
 
-- 
2.16.1


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

Re: [PATCH v5 5/5] build/clang: add a check whether the assembler supports .skip with labels

[Jan Beulich]

>>> On 20.02.18 at 15:10, <roger.pau@citrix.com> wrote:
> Or else disable the integrated assembler for assembly files. This is
> relevant for older clang versions which integrated assembler don't
> support .skip with labels.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
but pending the decision what to do about as-insn-check's name.

Jan

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