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

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

[Jordan Justen]

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
    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
    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
    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

[PATCH 2/2] EmulatorPkg/build.sh: Merge GCC toolchain detection from OVMF

[Jordan Justen]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
 EmulatorPkg/build.sh | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh
index 7f74391120..a196c3e9f3 100755
--- a/EmulatorPkg/build.sh
+++ b/EmulatorPkg/build.sh
@@ -93,6 +93,13 @@ case `uname` in
 
     gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
     case $gcc_version in
+      [1-3].*|4.[0-3].*)
+        echo OvmfPkg requires GCC4.4 or later
+        exit 1
+        ;;
+      4.4.*)
+        TARGET_TOOLS=GCC44
+        ;;
       4.5.*)
         TARGET_TOOLS=GCC45
         ;;
@@ -105,11 +112,11 @@ case `uname` in
       4.8.*)
         TARGET_TOOLS=GCC48
         ;;
-      4.9.*|4.1[0-9].*|5.*.*)
+      4.9.*|6.[0-2].*)
         TARGET_TOOLS=GCC49
         ;;
       *)
-        TARGET_TOOLS=GCC44
+        TARGET_TOOLS=GCC5
         ;;
     esac
     ;;
-- 
2.11.0

Re: [PATCH 2/2] EmulatorPkg/build.sh: Merge GCC toolchain detection from OVMF

[Laszlo Ersek]

On 06/02/17 01:56, Jordan Justen wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
>  EmulatorPkg/build.sh | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/EmulatorPkg/build.sh b/EmulatorPkg/build.sh
> index 7f74391120..a196c3e9f3 100755
> --- a/EmulatorPkg/build.sh
> +++ b/EmulatorPkg/build.sh
> @@ -93,6 +93,13 @@ case `uname` in
>  
>      gcc_version=$(gcc -v 2>&1 | tail -1 | awk '{print $3}')
>      case $gcc_version in
> +      [1-3].*|4.[0-3].*)
> +        echo OvmfPkg requires GCC4.4 or later

This should say EmulatorPkg, not OvmfPkg.

With that fixed:

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

Thanks
Laszlo

> +        exit 1
> +        ;;
> +      4.4.*)
> +        TARGET_TOOLS=GCC44
> +        ;;
>        4.5.*)
>          TARGET_TOOLS=GCC45
>          ;;
> @@ -105,11 +112,11 @@ case `uname` in
>        4.8.*)
>          TARGET_TOOLS=GCC48
>          ;;
> -      4.9.*|4.1[0-9].*|5.*.*)
> +      4.9.*|6.[0-2].*)
>          TARGET_TOOLS=GCC49
>          ;;
>        *)
> -        TARGET_TOOLS=GCC44
> +        TARGET_TOOLS=GCC5
>          ;;
>      esac
>      ;;
> 

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

[Ard Biesheuvel]

On 1 June 2017 at 23:56, Jordan Justen <jordan.l.justen@intel.com> wrote:

A commit log would be nice

> 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
>     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
>     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?

>     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
>

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

[Laszlo Ersek]

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
>>

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

[Jordan Justen]

On 2017-06-02 14:48:14, Laszlo Ersek wrote:
> 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:

How about this?

EmulatorPkg/Unix/Host: Add GCC5 CC/DLINK commands (for GCC >= 5)

These flags are based on the flags from the GCC5 toolchain in
tools_def.template. Since the GCC5 toolchain uses link-time
optimizations (LTO), we must compile and link the 'Host' files with
LTO enabled so we can link to other modules.

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>

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

[Ard Biesheuvel]

On 5 June 2017 at 18:33, Jordan Justen <jordan.l.justen@intel.com> wrote:
> On 2017-06-02 14:48:14, Laszlo Ersek wrote:
>> 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:
>
> How about this?
>
> EmulatorPkg/Unix/Host: Add GCC5 CC/DLINK commands (for GCC >= 5)
>
> These flags are based on the flags from the GCC5 toolchain in
> tools_def.template. Since the GCC5 toolchain uses link-time
> optimizations (LTO), we must compile and link the 'Host' files with
> LTO enabled so we can link to other modules.
>

Perfect.

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

[Laszlo Ersek]

On 06/06/17 13:10, Ard Biesheuvel wrote:
> On 5 June 2017 at 18:33, Jordan Justen <jordan.l.justen@intel.com> wrote:
>> On 2017-06-02 14:48:14, Laszlo Ersek wrote:
>>> 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:
>>
>> How about this?
>>
>> EmulatorPkg/Unix/Host: Add GCC5 CC/DLINK commands (for GCC >= 5)
>>
>> These flags are based on the flags from the GCC5 toolchain in
>> tools_def.template. Since the GCC5 toolchain uses link-time
>> optimizations (LTO), we must compile and link the 'Host' files with
>> LTO enabled so we can link to other modules.
>>
> 
> Perfect.
> 

+1