On 07/16/16 00:16, Ard Biesheuvel wrote: > Now that we switched to the __builtin_ms_va_list VA_LIST type for > GCC/X64, we can trust the compiler to do the right thing even under > optimization, and so we can enable -Os optimization all the way back > to GCC44, and drop the -D define that prevents the use of the __builtin > VA_LIST types. Note that this requires the -maccumulate-outgoing-args > switch as well. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > BaseTools/Conf/tools_def.template | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/BaseTools/Conf/tools_def.template > b/BaseTools/Conf/tools_def.template > index 2065fa34998f..a7da6741611d 100644 > --- a/BaseTools/Conf/tools_def.template > +++ b/BaseTools/Conf/tools_def.template > @@ -4353,7 +4353,7 @@ DEFINE GCC_AARCH64_RC_FLAGS = -I binary -O > elf64-littleaarch64 -B aarch64 > > DEFINE GCC44_ALL_CC_FLAGS = -g -fshort-wchar -fno-strict-aliasing > -Wall -Werror -Wno-array-bounds -ffunction-sections -fdata-sections -c > -include AutoGen.h -fno-common -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings > DEFINE GCC44_IA32_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) -m32 > -march=i586 -malign-double -fno-stack-protector -D EFI32 > -fno-asynchronous-unwind-tables > -DEFINE GCC44_X64_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) -m64 > -fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS > -mno-red-zone -Wno-address -mcmodel=large -fno-asynchronous-unwind-tables > +DEFINE GCC44_X64_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) -m64 > -fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" -Os > -maccumulate-outgoing-args -mno-red-zone -Wno-address -mcmodel=large > -fno-asynchronous-unwind-tables > DEFINE GCC44_IA32_X64_DLINK_COMMON = -nostdlib -n -q --gc-sections -z > common-page-size=0x20 > DEFINE GCC44_IA32_X64_ASLDLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_COMMON) > --entry ReferenceAcpiTable -u ReferenceAcpiTable > DEFINE GCC44_IA32_X64_DLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_COMMON) > --entry $(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Map > $(DEST_DIR_DEBUG)/$(BASE_NAME).map >
Before I embark on build-testing this series too with my "build farm", I'd like to point out this thread: http://thread.gmane.org/gmane.comp.bios.tianocore.devel/10741/focus=10961 Now, the assumption that -Os itself was causing the corruption has been laid to rest; we now know that the corruption was the product of the VA_LIST implementation, which is exactly what this series is replacing. So that's not why I'm pointing at the thread. However, I recall from the thread that -Os enables -fomit-frame-pointer, which might make source level debugging impossible (according to the GCC manual). Now, we're not big on source level debugging in GCC builds, at least right now, plus I also cannot claim that that -fomit-frame-pointer is never enabled *otherwise*. Much as I know -fomit-frame-pointer could be enabled with -O1, -O2, even with -O0?... I'd just like to avoid a setting that *guarantees* that source level debugging would be impossible or garbled. Ard, can you comment on that? ... Actually, just now I'm remembering something Scott explained to me: the difference between DEBUG, RELEASE, and NOOPT. Both DEBUG and RELEASE are supposed to be optimized (they differ in the compilation of DEBUG, DEBUG_CODE, ASSERT etc; not in optimization). NOOPT on the other hand is supposed to keep DEBUGs, but also disable optimization (for source level debugging). At the moment, we have no NOOPT settings for GCC. We only have RELEASE (in the "supposed" meaning of RELEASE), and DEBUG (which has, traditionally, stood for the NOOPT behavior actually). Version 1 of this patch set uses -O2 instead of -Os, but another difference is that v1 only added optimization to RELEASE. This version adds optimization (-Os) to DEBUG too (*) -- I guess in no small part because I expressed a wish for that? --, but it doesn't introduce a NOOPT target. I'm concerned that this might cause us to lose any usable source level debugging, even though our current "source level debugging" facility means a super contrived, out-of-tree gdb setup. (*) This is not my "discovery" of course, it's announced in the v2 blurb. I don't really know what to ask for / wish for :) I think introducing NOOPT might be a sizeable task, and it would even require changes to platforms (OvmfPkg and ArmVirtPkg minimally). So I don't feel good about asking Ard to add NOOPT as well. Instead, I admit that my suggestion (implied request?) in the v1 thread -- i.e., to add optimization to DEBUG -- broke the GCC toolchain tradition of DEBUG standing for NOOPT actually. I'm very sorry about that. :( In order to uphold the GCC toolchain tradition for DEBUG, should we add -Os (and whatever else -Os requires) to RELEASE only? Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
