Re: [Xen-devel] [PATCH v3 2/5] x86/clang: fix build with indirect thunks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.01.18 at 13:26, <roger.pau@xxxxxxxxxx> wrote:
> --- a/Config.mk
> +++ b/Config.mk
> @@ -157,17 +157,16 @@ 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,"insn",option-yes,option-no,FLAGS)
>  as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
> -                       | $(1) $(filter-out -M% %.d -include 
> %/include/xen/config.h,$(AFLAGS)) \
> +                       | $(1) $(filter-out -M% %.d -include 
> %/include/xen/config.h,$(5)) \

Despite this making for a larger diff, I don't think it makes sense to
add this new parameter as the last one. Natural would imo be (using
the provided example)

$(call as-insn cc,FLAGS,"insn",option-yes,option-no)

unless perhaps the flags can't be passed together with the build
tool name in the first place.

>                                -c -x c -o /dev/null - 2>&1),$(4),$(3))
> -

Please don't remove this blank line.

> --- a/xen/include/asm-x86/asm_defns.h
> +++ b/xen/include/asm-x86/asm_defns.h
> @@ -16,9 +16,31 @@
>  #ifdef __ASSEMBLY__
>  # include <asm/indirect_thunk_asm.h>
>  #else
> -asm ( "\t.equ CONFIG_INDIRECT_THUNK, "
> -      __stringify(IS_ENABLED(CONFIG_INDIRECT_THUNK)) );
> -asm ( "\t.include \"asm/indirect_thunk_asm.h\"" );
> +/*
> + * NB: for clang's integrated assembler the macro must be included in every
> + * asm() instance where it's used. For gcc/gas it only needs to be included
> + * once per file. Note that even guarding the indirect_thunk_asm.h with
> + * something like:
> + *
> + * .ifndef _X86_INDIRECT_THUNK_ASM_H_
> + * .equ _X86_INDIRECT_THUNK_ASM_H_, 1
> + * ...
> + * .endif
> + *
> + * Doesn't work with clang because it will keep the .equ defined between 
> asm()

doesn't

> + * instances, but will forget the .macro. This has been reported upstream:
> + * https://bugs.llvm.org/show_bug.cgi?id=36110 
> + */

Ugly. One more remark concerning the potential (longer term)
negative aspects of the chose approach: Producing equates
and defining macros in the middle of other code is probably
fine, but what if other things get added to the header? Not
everything commonly done in a header (even if that's an
assembly only one) is necessarily benign to the surrounding
code.

IOW - I'm not convinced it is desirable to be able to use the
integrated assembler, if there are such significant quirks to
work around.

Jan


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