Re: [PATCH v4 0/7] BaseTools: add support for GCC5 in LTO mode

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

On 29 July 2016 at 22:33, Jordan Justen <jordan.l.justen@intel.com> wrote:
> On 2016-07-28 23:09:15, Ard Biesheuvel wrote:
>> On 29 July 2016 at 06:47, Gao, Liming <liming.gao@intel.com> wrote:
>> > Ard:
>> >   Thanks for your update. I have some comments for them.
>> > 1) It uses GCC as Link for GCC44-GCC49. Have you done verification on them? I verify GCC49 in OVMFIa32X64 platform. It works.
>>
>> Yes, I tested all of them.
>>
>
> I tested GCC44 (X64) on an old live-cd where GCC 4.4 was the supported
> version. OVMF built and booted UEFI Linux. Therefore, I don't think we
> are likely to have a major issue with GCC44-GCC49.
>
> I also tested GCC49 and GCC5 with X64 on a system with GCC 5.4. It
> built and booted UEFI Linux.
>

OK, so you are not seeing the issue Liming reported when using the
GCC49 toolchain tag with GCC 5.4?

In that case, I am inclined to diagnose Liming's issue as a GCC 5.x
regression that was fixed in 5.4. Adding the GOT handling to GenFw
would still be possible, of course, but GOT based symbol references
are sub-optimal so we should try to avoid them imo. If GCC 4.x does
not emit any such references with protected visibility enabled, and
nor does GCC 5.4, I think it it reasonable to require that GCC 5.3
users simply use the GCC5 profile (with LTO), and switch to either 4.9
or 5.4 if they want to use the GCC49 profile instead.

> Can you add something like 'added GCC5 toolchain' into the subject for
> patch 7? I really think that a patch that adds a new toolchain should
> make it immediately obvious in the subject line.
>

Of course, that makes sense.

> 3, 4 & 7 Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
>
> The others, Acked-by: Jordan Justen <jordan.l.justen@intel.com>
>

Thanks,
Ard.