[edk2] [PATCH v2 5/5] BaseTools AARCH64: add -mstrict-align to all AARCH64 GCC flavors

2015-12-31 Thread Ard Biesheuvel 
GCC for AARCH64 recognizes byte swapping load and store sequences
and may replace them with wider loads or stores combined with rev
instructions. In some cases (i.e., with GCC version 5 and later)
this may result in unaligned accesses, which are not allowed before
we turn the MMU on.

So move the -mstrict-align compiler switch to the shared define for
all AARCH64 GCC versions.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Liming Gao 
Reviewed-by: Leif Lindholm 
---
 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 096f00d48e64..82fae766e605 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4323,7 +4323,7 @@ DEFINE GCC_IA32_CC_FLAGS   = 
DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double -
 DEFINE GCC_X64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mno-red-zone 
-Wno-address -mno-stack-arg-probe
 DEFINE GCC_IPF_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
-minline-int-divide-min-latency
 DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-mabi=aapcs -fno-short-enums -save-temps -fsigned-char -ffunction-sections 
-fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
-DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -save-temps -fverbose-asm -fsigned-char  -ffunction-sections 
-fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
-fno-asynchronous-unwind-tables
+DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -save-temps -fverbose-asm -fsigned-char  -ffunction-sections 
-fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
-fno-asynchronous-unwind-tables -mstrict-align
 DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
 DEFINE GCC_DLINK2_FLAGS_COMMON = 
--script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
 DEFINE GCC_IA32_X64_DLINK_COMMON   = DEF(GCC_DLINK_FLAGS_COMMON) --gc-sections
@@ -5188,7 +5188,7 @@ DEFINE CLANG35_AARCH64_TARGET= -target 
aarch64-none-linux-gnu
 
 DEFINE CLANG35_WARNING_OVERRIDES = -Wno-parentheses-equality 
-Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare 
-Wno-empty-body
 DEFINE CLANG35_ARM_CC_FLAGS  = DEF(GCC_ARM_CC_FLAGS) 
DEF(CLANG35_ARM_TARGET) -mstrict-align -mllvm -arm-use-movt=0 
DEF(CLANG35_WARNING_OVERRIDES)
-DEFINE CLANG35_AARCH64_CC_FLAGS  = DEF(GCC_AARCH64_CC_FLAGS) 
DEF(CLANG35_AARCH64_TARGET) -mcmodel=small -mstrict-align 
DEF(CLANG35_WARNING_OVERRIDES)
+DEFINE CLANG35_AARCH64_CC_FLAGS  = DEF(GCC_AARCH64_CC_FLAGS) 
DEF(CLANG35_AARCH64_TARGET) -mcmodel=small DEF(CLANG35_WARNING_OVERRIDES)
 
 ##
 # CLANG35 ARM definitions
-- 
2.5.0

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 5/5] BaseTools AARCH64: add -mstrict-align to all AARCH64 GCC flavors

2015-12-31 Thread Ard Biesheuvel 
On 31 December 2015 at 13:57, Ard Biesheuvel  wrote:
> GCC for AARCH64 recognizes byte swapping load and store sequences
> and may replace them with wider loads or stores combined with rev
> instructions. In some cases (i.e., with GCC version 5 and later)
> this may result in unaligned accesses, which are not allowed before
> we turn the MMU on.
>
> So move the -mstrict-align compiler switch to the shared define for
> all AARCH64 GCC versions.
>

Unfortunately, it turns out that the code size increase is
non-negligible (about 2% for the tiny model release build of
ArmVirtQemu, as an example), and may have a performance impact as
well.

Instead, we would like to add something like

[BuildOptions.common.EDKII.BASE, BuildOptions.common.EDKII.SEC,
BuildOptions.common.EDKII.PEI_CORE, BuildOptions.common.EDKII.PEIM]
  GCC:*_*_AARCH64_CC_FLAGS = -mstrict-align

so that only code that may execute with the MMU off gets the
-mstrict-align treatment. However, it would be *much* more useful if
we could add this to tools_def.txt as a global module type specific
override rather than something each platform needs to define in its
.DSC.

@Liming, Yonghong: have you ever considered adding a feature like this
to BaseTools? Could you comment on the feasibility, i.e., would it be
a huge amount of work to implement this?

Thanks,
Ard.



> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> Reviewed-by: Liming Gao 
> Reviewed-by: Leif Lindholm 
> ---
>  BaseTools/Conf/tools_def.template | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/BaseTools/Conf/tools_def.template 
> b/BaseTools/Conf/tools_def.template
> index 096f00d48e64..82fae766e605 100644
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -4323,7 +4323,7 @@ DEFINE GCC_IA32_CC_FLAGS   = 
> DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double -
>  DEFINE GCC_X64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mno-red-zone 
> -Wno-address -mno-stack-arg-probe
>  DEFINE GCC_IPF_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
> -minline-int-divide-min-latency
>  DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
> -mabi=aapcs -fno-short-enums -save-temps -fsigned-char -ffunction-sections 
> -fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
> -DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
> -fno-short-enums -save-temps -fverbose-asm -fsigned-char  -ffunction-sections 
> -fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
> -fno-asynchronous-unwind-tables
> +DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
> -fno-short-enums -save-temps -fverbose-asm -fsigned-char  -ffunction-sections 
> -fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
> -fno-asynchronous-unwind-tables -mstrict-align
>  DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
>  DEFINE GCC_DLINK2_FLAGS_COMMON = 
> --script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
>  DEFINE GCC_IA32_X64_DLINK_COMMON   = DEF(GCC_DLINK_FLAGS_COMMON) 
> --gc-sections
> @@ -5188,7 +5188,7 @@ DEFINE CLANG35_AARCH64_TARGET= -target 
> aarch64-none-linux-gnu
>
>  DEFINE CLANG35_WARNING_OVERRIDES = -Wno-parentheses-equality 
> -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare 
> -Wno-empty-body
>  DEFINE CLANG35_ARM_CC_FLAGS  = DEF(GCC_ARM_CC_FLAGS) 
> DEF(CLANG35_ARM_TARGET) -mstrict-align -mllvm -arm-use-movt=0 
> DEF(CLANG35_WARNING_OVERRIDES)
> -DEFINE CLANG35_AARCH64_CC_FLAGS  = DEF(GCC_AARCH64_CC_FLAGS) 
> DEF(CLANG35_AARCH64_TARGET) -mcmodel=small -mstrict-align 
> DEF(CLANG35_WARNING_OVERRIDES)
> +DEFINE CLANG35_AARCH64_CC_FLAGS  = DEF(GCC_AARCH64_CC_FLAGS) 
> DEF(CLANG35_AARCH64_TARGET) -mcmodel=small DEF(CLANG35_WARNING_OVERRIDES)
>
>  ##
>  # CLANG35 ARM definitions
> --
> 2.5.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel