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