Re: [PATCH] BaseTools/tools_def: add CLANG38 LTO versions for AARCH64 and ARM

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 6 December 2017 at 14:12, Laszlo Ersek <lersek@redhat.com> wrote:
> On 12/06/17 12:39, Ard Biesheuvel wrote:
>> Extend the CLANG38 toolchain definition so it can be used for
>> ARM and AARCH64 as well. Note that this requires llvm-ar and
>> the LLVMgold.so linker plugin.
>>
>> In preparation of doing the same for GCC5, this toolchain version
>> also departs from the custom of using -O0 for DEBUG builds, which
>> makes them needlessly slow. Instead, let's add a NOOPT flavor as
>> well, and enable optimization for DEBUG like the other architectures
>> do. (Note that this will require some trivial changes to the platform
>> description files)
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  BaseTools/Conf/tools_def.template | 98 +++++++++++++++++++-
>>  1 file changed, 95 insertions(+), 3 deletions(-)
>>
>> diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
>> index 91b135c2e569..6ee720d7586e 100755
>> --- a/BaseTools/Conf/tools_def.template
>> +++ b/BaseTools/Conf/tools_def.template
>> @@ -399,8 +399,8 @@ DEFINE DTC_BIN                 = ENV(DTC_PREFIX)dtc
>>  #                               Intel(r) ACPI Compiler from
>>  #                               https://acpica.org/downloads
>>  #   CLANG38  -Linux-  Requires:
>> -#                             Clang v3.8, LLVMgold plugin and GNU binutils 2.26 targeting x86_64-linux-gnu
>> -#                             Clang v3.9 or later, LLVMgold plugin and GNU binutils 2.28 targeting x86_64-linux-gnu
>> +#                             Clang v3.8, LLVMgold plugin and GNU binutils 2.26 targeting x86_64-linux-gnu, aarch64-linux-gnu or arm-linux-gnueabi
>> +#                             Clang v3.9 or later, LLVMgold plugin and GNU binutils 2.28 targeting x86_64-linux-gnu, aarch64-linux-gnu or arm-linux-gnueabi
>>  #                        Optional:
>>  #                             Required to build platforms or ACPI tables:
>>  #                               Intel(r) ACPI Compiler from
>> @@ -5652,6 +5652,7 @@ RELEASE_CLANG35_AARCH64_CC_FLAGS = DEF(CLANG35_AARCH64_CC_FLAGS) $(ARCHCC_FLAGS)
>>  *_CLANG38_*_MAKE_PATH               = make
>>  *_CLANG38_*_*_DLL                   = ENV(CLANG38_DLL)
>>  *_CLANG38_*_ASL_PATH                = DEF(UNIX_IASL_BIN)
>> +*_CLANG38_*_DTC_PATH                = DEF(DTC_BIN)
>>
>>  *_CLANG38_*_APP_FLAGS               =
>>  *_CLANG38_*_ASL_FLAGS               = DEF(IASL_FLAGS)
>> @@ -5663,7 +5664,8 @@ DEFINE CLANG38_X64_PREFIX           = ENV(CLANG38_BIN)
>>  DEFINE CLANG38_IA32_TARGET          = -target i686-pc-linux-gnu
>>  DEFINE CLANG38_X64_TARGET           = -target x86_64-pc-linux-gnu
>>
>> -DEFINE CLANG38_ALL_CC_FLAGS         = DEF(GCC44_ALL_CC_FLAGS) -Wno-empty-body -fno-stack-protector -mms-bitfields -Wno-address -Wno-shift-negative-value -Wno-parentheses-equality -Wno-unknown-pragmas -Wno-tautological-constant-out-of-range-compare -Wno-incompatible-library-redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -msoft-float -mno-implicit-float  -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno-tautological-compare -Wno-unknown-warning-option -Wno-varargs
>> +DEFINE CLANG38_WARNING_OVERRIDES    = -Wno-parentheses-equality -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare -Wno-empty-body  -Wno-varargs
>> +DEFINE CLANG38_ALL_CC_FLAGS         = DEF(GCC44_ALL_CC_FLAGS) DEF(CLANG38_WARNING_OVERRIDES) -fno-stack-protector -mms-bitfields -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas -Wno-incompatible-library-redeclaration -fno-asynchronous-unwind-tables -mno-sse -mno-mmx -msoft-float -mno-implicit-float  -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -funsigned-char -fno-ms-extensions -Wno-null-dereference -Wno-unknown-warning-option
>
> At a quite superficial look, the patch seems OK to me. I'm just curious
> about one thing: what decides if a -Wno... option goes into
> CLANG38_WARNING_OVERRIDES? You left some -Wno... options out of it (kept
> them explicitly in the CLANG38_ALL_CC_FLAGS define).
>

I split off some warning overrides that are shared between all
architectures, and left the x86 only ones in CLANG38_ALL_CC_FLAGS

> ... It seems like the CLANG38_ALL_CC_FLAGS is not used for the ARM /
> AARCH64 toolchains. "ALL" becomes sort of a misnomer then. Is that OK?
>

Yes. In tools_def land, ALL still means 'both IA32 and X64', and I
have wasted too many cycles on trying to refactor the GCC toolchains
to care about it.