diff mbox series

[XEN,8/8] xen: Move GCC_HAS_VISIBILITY_ATTRIBUTE to Kconfig and common

Message ID 20191212182740.2190199-9-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series xen: Kconfig update with few extra | expand

Commit Message

Anthony PERARD Dec. 12, 2019, 6:27 p.m. UTC
The check for $(CC) -fvisibility=hidden is done by both arm and x86,
so the patch also move the check to the common area.

The check doesn't check if $(CC) is gcc, and clang can accept that
option as well, so s/GCC/CC/ is done to the define name.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 xen/Kconfig                | 4 ++++
 xen/arch/arm/Rules.mk      | 4 ----
 xen/arch/x86/Rules.mk      | 5 -----
 xen/include/xen/compiler.h | 2 +-
 4 files changed, 5 insertions(+), 10 deletions(-)

Comments

Andrew Cooper Dec. 12, 2019, 7:04 p.m. UTC | #1
On 12/12/2019 18:27, Anthony PERARD wrote:
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index ff6c0f5cdd18..8c846261d241 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -78,7 +78,7 @@
>  #define __must_be_array(a) \
>    BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof(&a[0])))
>  
> -#ifdef GCC_HAS_VISIBILITY_ATTRIBUTE
> +#ifdef CONFIG_CC_HAS_VISIBILITY_ATTRIBUTE
>  /* Results in more efficient PIC code (no indirections through GOT or PLT). */
>  #pragma GCC visibility push(hidden)

(I realise we are getting into archaeology, but) Why do we have this as
a pragma gcc?

Surely it would be simpler to just feed -fvisibility=hidden into CFLAGS?

Ack for the overall change, but I expect we can clean this up a bit more
as well.

~Andrew
Jan Beulich Dec. 13, 2019, 11:27 a.m. UTC | #2
On 12.12.2019 20:04, Andrew Cooper wrote:
> On 12/12/2019 18:27, Anthony PERARD wrote:
>> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
>> index ff6c0f5cdd18..8c846261d241 100644
>> --- a/xen/include/xen/compiler.h
>> +++ b/xen/include/xen/compiler.h
>> @@ -78,7 +78,7 @@
>>  #define __must_be_array(a) \
>>    BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof(&a[0])))
>>  
>> -#ifdef GCC_HAS_VISIBILITY_ATTRIBUTE
>> +#ifdef CONFIG_CC_HAS_VISIBILITY_ATTRIBUTE
>>  /* Results in more efficient PIC code (no indirections through GOT or PLT). */
>>  #pragma GCC visibility push(hidden)
> 
> (I realise we are getting into archaeology, but) Why do we have this as
> a pragma gcc?
> 
> Surely it would be simpler to just feed -fvisibility=hidden into CFLAGS?

No, as per the (admittedly not very explicit) commit message of the
change introducing it. Quoting gcc documentation for making it more
obvious:

"extern declarations are not affected by ‘-fvisibility’, so a lot of code
 can be recompiled with ‘-fvisibility=hidden’ with no modifications.
 However, this means that calls to extern functions with no explicit
 visibility use the PLT, so it is more effective to use
 __attribute ((visibility)) and/or #pragma GCC visibility to tell the
 compiler which extern declarations should be treated as hidden."

Jan
diff mbox series

Patch

diff --git a/xen/Kconfig b/xen/Kconfig
index 9f6512d65b08..fc49f4c30a29 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -24,6 +24,10 @@  config CLANG_VERSION
 	int
 	default $(shell,$(BASEDIR)/scripts/clang-version.sh $(CC))
 
+# -fvisibility=hidden reduces -fpic cost, if it's available
+config CC_HAS_VISIBILITY_ATTRIBUTE
+	def_bool $(cc-option,-fvisibility=hidden)
+
 source "arch/$(SRCARCH)/Kconfig"
 
 config DEFCONFIG_LIST
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 3d9a0ed357bc..022a3a6f82ba 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -18,10 +18,6 @@  CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-a15
 CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
 CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
 
-ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
-CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
-endif
-
 EARLY_PRINTK := n
 
 ifeq ($(CONFIG_DEBUG),y)
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index b98e14e28c5a..e69b8e697cc0 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -65,11 +65,6 @@  CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
 # SSE setup for variadic function calls.
 CFLAGS += -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
 
-# -fvisibility=hidden reduces -fpic cost, if it's available
-ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
-CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
-endif
-
 # Compile with thunk-extern, indirect-branch-register if avaiable.
 ifeq ($(CONFIG_INDIRECT_THUNK),y)
 CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index ff6c0f5cdd18..8c846261d241 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -78,7 +78,7 @@ 
 #define __must_be_array(a) \
   BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof(&a[0])))
 
-#ifdef GCC_HAS_VISIBILITY_ATTRIBUTE
+#ifdef CONFIG_CC_HAS_VISIBILITY_ATTRIBUTE
 /* Results in more efficient PIC code (no indirections through GOT or PLT). */
 #pragma GCC visibility push(hidden)
 #endif