[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 2/4] x86/clang: restore integrated assembler usage with indirect thunks



>>> On 19.02.18 at 15:16, <roger.pau@xxxxxxxxxx> wrote:
> --- 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)

Why lowercase cc and uppercase FLAGS? Along the lines of the
usage comment ahead of as-insn-check both should be uppercase.

> --- 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.
> +    ifeq ($(call as-insn,$(CC) $(CFLAGS),".include 
> \"asm/indirect_thunk_asm.h\"",y,n),n)

Is there anything keeping you from using the slightly less ugly to use
as-insn-check here? Oh, it's apparently that you want to use CFLAGS,
not AFLAGS. I wonder whether the other as-insn-check uses wouldn't
better have CFLAGS passed too. Otherwise please clarify why the
other construct can't be used by extending the comment.

> +        CFLAGS += -no-integrated-as
> +    # Check whether clang keeps .macro-s between asm()-s:
> +    # https://bugs.llvm.org/show_bug.cgi?id=36110 
> +    else ifeq ($(if $(shell echo 'void _(void) { asm volatile ( ".macro 
> FOO\n.endm" ); \

Careful with this; ./README still says "GNU make 3.80 or later" and
iirc 3.80 doesn't support "else if..." on a single line.

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.