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

From: B Cran <bruce@cran.org.uk>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Laszlo Ersek <lersek@redhat.com>,
	"edk2-devel@lists.01.org" <edk2-devel@ml01.01.org>,
	Liming Gao <liming.gao@intel.com>
Subject: Re: [Patch V2] BaseTools: support the NOOPT target with the GCC tool chains
Date: Wed, 5 Oct 2016 16:46:56 -0600	[thread overview]
Message-ID: <F92208FA-9B20-4CAF-BAC9-BEAABD7DBD05@cran.org.uk> (raw)
In-Reply-To: <CAKv+Gu80f-Qwb=LQSUuDOxdO-i7O0_zERR4-1_cY8WkKqa+JAw@mail.gmail.com>

Tested-by: Bruce Cran <bruce.cran@gmail.com>
> On Oct 5, 2016, at 12:06 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
>> On 5 October 2016 at 18:56, Laszlo Ersek <lersek@redhat.com> wrote:
>>> 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.
>> 
> 
> OK, fair enough. Leif and I can look into this in the future.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel