Re: [PATCH 1/2] EmulatorPkg/Unix/Host: Add GCC5 CC/DLINK commands (for GCC >= 5)

[Laszlo Ersek <lersek@redhat.com>]

On 06/02/17 11:03, Ard Biesheuvel wrote:
> On 1 June 2017 at 23:56, Jordan Justen <jordan.l.justen@intel.com> wrote:
> 
> A commit log would be nice

I agree. FWIW:

> 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  EmulatorPkg/Unix/Host/Host.inf | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/EmulatorPkg/Unix/Host/Host.inf b/EmulatorPkg/Unix/Host/Host.inf
>> index d83857a1fd..33990fc20e 100644
>> --- a/EmulatorPkg/Unix/Host/Host.inf
>> +++ b/EmulatorPkg/Unix/Host/Host.inf
>> @@ -122,6 +122,7 @@
>>     GCC:*_*_IA32_ASM_FLAGS == -m32 -c -x assembler -imacros $(DEST_DIR_DEBUG)/AutoGen.h
>>
>>     GCC:*_*_X64_DLINK_FLAGS == -o $(BIN_DIR)/Host -m64 -L/usr/X11R6/lib
>> +   GCC:*_GCC5_X64_DLINK_FLAGS == -flto -o $(BIN_DIR)/Host -m64 -L/usr/X11R6/lib

This looks good to me.

>>     GCC:*_*_X64_CC_FLAGS == -m64 -g -fshort-wchar -fno-strict-aliasing -Wall -malign-double -idirafter/usr/include -c -include $(DEST_DIR_DEBUG)/AutoGen.h -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings

(Mark this line.)

>>     GCC:*_GCC44_X64_CC_FLAGS = "-DEFIAPI=__attribute__((ms_abi))"
>>     GCC:*_GCC45_X64_CC_FLAGS = "-DEFIAPI=__attribute__((ms_abi))"
>> @@ -129,6 +130,7 @@
>>     GCC:*_GCC47_X64_CC_FLAGS = "-DEFIAPI=__attribute__((ms_abi))"
>>     GCC:*_GCC48_X64_CC_FLAGS = "-DEFIAPI=__attribute__((ms_abi))"
>>     GCC:*_GCC49_X64_CC_FLAGS = "-DEFIAPI=__attribute__((ms_abi))"
>> +   GCC:*_GCC5_X64_CC_FLAGS = "-DEFIAPI=__attribute__((ms_abi))" -flto -DUSING_LTO -Os
> 
> Single = is additive, so is it necessary to add the -Os as well?

I think so; namely, the line I highlighted above uses ==, so it
overwrites all other settings in effect earlier. *That* line does not
have -Os, so when *this* line applies to the target / toolchain / arch
triplet, and -Os is appended, we don't end up with two -Os optons, only one.

If my argument is correct (and a hopefully similar commit message is added):

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> 
>>     GCC:*_*_X64_PP_FLAGS == -m64 -E -x assembler-with-cpp -include $(DEST_DIR_DEBUG)/AutoGen.h
>>     GCC:*_*_X64_ASM_FLAGS == -m64 -c -x assembler -imacros $(DEST_DIR_DEBUG)/AutoGen.h
>>
>> --
>> 2.11.0
>>