[Xen-devel] [PATCH for-4.13] x86/clang: move and fix .skip check

Roger Pau Monne posted 1 patch 4 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20191114095927.83723-1-roger.pau@citrix.com
xen/Rules.mk          | 7 -------
xen/arch/x86/Rules.mk | 5 +++++
2 files changed, 5 insertions(+), 7 deletions(-)
[Xen-devel] [PATCH for-4.13] x86/clang: move and fix .skip check
Posted by Roger Pau Monne 4 years, 5 months ago
.skip is only used by x86 code, so place the clang .skip with labels
check in x86/Rules.mk instead of the top level Rules.mk. While there
also fix an issue with it by removing the '\n' which triggers the
following error:

<stdin>:1:31: error: missing terminating '"' character [-Werror,-Winvalid-pp-token]
void _(void) { asm volatile ( ".L0:
                              ^
<stdin>:1:31: error: expected string literal in 'asm'
<stdin>:3:18: error: missing terminating '"' character [-Werror,-Winvalid-pp-token]
.skip (.L1 - .L0)" ); }
                 ^
<stdin>:3:24: error: expected ')'
.skip (.L1 - .L0)" ); }
                       ^
<stdin>:1:29: note: to match this '('
void _(void) { asm volatile ( ".L0:
                            ^
<stdin>:3:24: error: expected '}'
.skip (.L1 - .L0)" ); }
                       ^
<stdin>:1:14: note: to match this '{'
void _(void) { asm volatile ( ".L0:
             ^
5 errors generated.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Juergen Gross <jgross@suse.com>
---
NB: will need rebasing on top of Jan's patch (or the other way
around).
---
 xen/Rules.mk          | 7 -------
 xen/arch/x86/Rules.mk | 5 +++++
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 3090ea7828..d1c060c3cf 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -76,13 +76,6 @@ endif
 
 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
-ifeq ($(clang),y)
-$(call as-option-add,CFLAGS,CC,".L0:\n.L1:\n.skip (.L1 - .L0)",,\
-                     -no-integrated-as)
-endif
-
 ALL_OBJS := $(ALL_OBJS-y)
 
 # Get gcc to generate the dependencies for us.
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 659ac3d83e..328bbfea9d 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -83,4 +83,9 @@ $(call as-option-add,CFLAGS,CC,".include \"asm/indirect_thunk_asm.h\"",,\
 $(call as-option-add,CFLAGS,CC,\
                      ".macro FOO\n.endm\"); asm volatile (\".macro FOO\n.endm",\
                      -no-integrated-as)
+
+# Older clang's built-in assembler doesn't understand .skip with labels:
+# https://bugs.llvm.org/show_bug.cgi?id=27369
+$(call as-option-add,CFLAGS,CC,".L0: .L1: .skip (.L1 - .L0)",,\
+                     -no-integrated-as)
 endif
-- 
2.24.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.13] x86/clang: move and fix .skip check
Posted by Jan Beulich 4 years, 5 months ago
On 14.11.2019 10:59, Roger Pau Monne wrote:
> .skip is only used by x86 code, so place the clang .skip with labels
> check in x86/Rules.mk instead of the top level Rules.mk. While there
> also fix an issue with it by removing the '\n' which triggers the
> following error:
> 
> <stdin>:1:31: error: missing terminating '"' character [-Werror,-Winvalid-pp-token]
> void _(void) { asm volatile ( ".L0:
>                               ^
> <stdin>:1:31: error: expected string literal in 'asm'
> <stdin>:3:18: error: missing terminating '"' character [-Werror,-Winvalid-pp-token]
> .skip (.L1 - .L0)" ); }
>                  ^
> <stdin>:3:24: error: expected ')'
> .skip (.L1 - .L0)" ); }
>                        ^
> <stdin>:1:29: note: to match this '('
> void _(void) { asm volatile ( ".L0:
>                             ^
> <stdin>:3:24: error: expected '}'
> .skip (.L1 - .L0)" ); }
>                        ^
> <stdin>:1:14: note: to match this '{'
> void _(void) { asm volatile ( ".L0:
>              ^
> 5 errors generated.

As said on the other thread - I'm afraid there's more to this difference
in un-escaping between your and my environments. I agree the newlines
aren't needed here at all, so I'd be fine to give my R-b, but we need to
fully understand the differences in observed behavior anyway.

Additionally I wonder whether you wouldn't better retain the original
sequence of checks, by placing the check you move at the beginning of
the "ifeq ($(clang),y)" block in x86/Rules.mk instead of at the end.
That'll (iirc) also better represent the history of the addition of
these checks (also demonstrated by the reference bug numbers).

Jan

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