Re: [Patch V2] BaseTools: support the NOOPT target with the GCC tool chains

[Laszlo Ersek <lersek@redhat.com>]

On 10/05/16 18:06, Ard Biesheuvel wrote:
> On 5 October 2016 at 15:48, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 10/05/16 03:30, Yonghong Zhu wrote:
>>> Update the tools_def.template to add NOOPT support with GCC tool chains.
>>>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com>
>>> ---
>>>  BaseTools/Conf/tools_def.template | 28 ++++++++++++++++++++++++++++
>>>  1 file changed, 28 insertions(+)
>>
>> I thought I understood what was going on, but apparently I was wrong
>> about that.
>>
>> In this patch, we add or modify:
>> - NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG -- okay
>> - NOOPT_GCC*_(IA32|X64|ARM|AARCH64)_CC_FLAGS -- okay
>>
>> So that part is fine with me. But then we also add / modify:
>> - NOOPT_GCC(49|5)_AARCH64_DLINK_(FLAGS|XIPFLAGS)
>> - NOOPT_GCC5_ARM_DLINK_FLAGS
>>
>> First I thought the latter set of changes was unnecessary, because "ld"
>> didn't use "-O". I checked the manual, and I was wrong: "ld" does know /
>> use "-O". So those changes are fine, I guess.
>>
> 
> Yes, especially under LTO, in which case code generation is performed
> during the link stage, which should adhere to the same rules as the
> compiler. This not only applies to -O, but also to things like
> -march/-mcpu and -mstrict-align. This is why we pass all CFLAGS to the
> linker for the GCC5 LTO builds.
> 
>> But then: is the patch *complete*? Because I can see some more DLINK
>> stuff, for IA32 and X64 (not just ARM and AARCH64). Is it okay to ignore
>> those? For example:
>>
>> *_GCC5_IA32_DLINK_FLAGS          = DEF(GCC5_IA32_X64_DLINK_FLAGS) -Os
>> -Wl,-m,elf_i386,--oformat=elf32-i386
>>
>>
>> *_GCC5_X64_DLINK_FLAGS           = DEF(GCC5_X64_DLINK_FLAGS) -Os
>>
>> Where GCC5_X64_DLINK_FLAGS and GCC5_IA32_X64_DLINK_FLAGS even include
>> -flto. (I don't know if "-flto" hampers source level debugging or not.)
>>
> 
> The GCC man page documents -flto as being a bad idea, i.e.,
> 
> """
> Link-time optimization does not work well with generation of debugging
> information.  Combining -flto with -g is currently experimental and
> expected to produce unexpected results.
> """
> 
> (which raises a philosophical question as well, i.e., to which extent
> expected unexpected results are still unexpected results. But I
> digress ...)
> 
> Another note: the DEBUG build for ARM and AARCH64 is essentially NOOPT
> already, not DEBUG. How does this patch intend to deal with that?

It just copies the DEBUG settings to NOOPT (via deep copy, not by
reference). I believe that's OK.

Thanks
Laszlo