| Message ID | 1497984234-19871-2-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 9ba8baae15caa1bdc969c451ce96e82d8de825ac |
| Headers | show |
| Series | [edk2,1/2] BaseTools/tools_def GCC: ARM/AARCH64: replace -save-temps with -pipe | expand |
On 06/20/17 20:43, Ard Biesheuvel wrote: > On AARCH64, any code that may execute with the MMU off needs to be built > with -mstrict-align, given that unaligned accesses are not allowed unless > the MMU is enabled. This does not only affect SEC and PEI modules, but > also static libraries of the BASE type, which may be linked into such > modules, as well as into modules of other types. As it turns out, the > presence of -mstrict-align is reflected in the internal representations > of the types defined in those libraries. > > When -fstrict-aliasing is passed to GCC, it assumes that pointers to > objects of different types cannot refer to the same memory location, and > attempts to exploit this fact when optimizing the code. Since such > assumptions are only valid under very strict conditions which are not > guaranteed to be met in EDK2, we disable this optimization by passing > -fno-strict-aliasing by default. > > When LTO is in effect, this applies equally to the code generation that > may occur at link time, which is why the linker warns about unexpected > differences in type definitions between the intermediate representations > that are present in the object files being linked. This may result in > warnings such as the one below, even if -fno-strict-aliasing is used: > > MdePkg/Include/Library/BaseLib.h:1712:1: > warning: type of 'StrToGuid' does not match original declaration > [-Wlto-type-mismatch] > StrToGuid ( > ^ > MdePkg/Library/BaseLib/SafeString.c:1506:1: > note: 'StrToGuid' was previously declared here > StrToGuid ( > ^ > MdePkg/Library/BaseLib/SafeString.c:1506:1: > note: code may be misoptimized unless -fno-strict-aliasing is used > > This warning is inadvertently triggered when linking BASE libraries built > with -mstrict-align into modules of types other than SEC or PEI, since the > types are subtly different, even though the use of code that maintains > strict alignment in a module that does not care about this is unlikely to > cause problems. And even if it did, it would still only affect code built > with -fstrict-aliasing enabled, which we disable unconditionally. So let's > just silence the warning by passing -Wno-lto-type-mismatch. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > 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 7a58ce365ed2..9a3ba9defb12 100755 > --- a/BaseTools/Conf/tools_def.template > +++ b/BaseTools/Conf/tools_def.template > @@ -5407,7 +5407,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS > DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 > > RELEASE_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -mcmodel=tiny -fomit-frame-pointer > -RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 > +RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch > > NOOPT_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small > NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0 > Acked-by: Laszlo Ersek <lersek@redhat.com> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Tue, Jun 20, 2017 at 08:43:54PM +0200, Ard Biesheuvel wrote: > On AARCH64, any code that may execute with the MMU off needs to be built > with -mstrict-align, given that unaligned accesses are not allowed unless > the MMU is enabled. This does not only affect SEC and PEI modules, but > also static libraries of the BASE type, which may be linked into such > modules, as well as into modules of other types. As it turns out, the > presence of -mstrict-align is reflected in the internal representations > of the types defined in those libraries. > > When -fstrict-aliasing is passed to GCC, it assumes that pointers to > objects of different types cannot refer to the same memory location, and > attempts to exploit this fact when optimizing the code. Since such > assumptions are only valid under very strict conditions which are not > guaranteed to be met in EDK2, we disable this optimization by passing > -fno-strict-aliasing by default. Just a comment - you don't necessarily have to update this excellent commit mesage, but: -fstrict-aliasing is GCC default, because it is a restriction in the C language. Because it's a bit non-obvious, things can go hilariously wrong in very non-obvious ways, and the potential optimization gains are ulikely to be generally relevant, -fno-strict-aliasing is a sensible thing to always have set (like we do). > When LTO is in effect, this applies equally to the code generation that > may occur at link time, which is why the linker warns about unexpected > differences in type definitions between the intermediate representations > that are present in the object files being linked. This may result in > warnings such as the one below, even if -fno-strict-aliasing is used: > > MdePkg/Include/Library/BaseLib.h:1712:1: > warning: type of 'StrToGuid' does not match original declaration > [-Wlto-type-mismatch] > StrToGuid ( > ^ > MdePkg/Library/BaseLib/SafeString.c:1506:1: > note: 'StrToGuid' was previously declared here > StrToGuid ( > ^ > MdePkg/Library/BaseLib/SafeString.c:1506:1: > note: code may be misoptimized unless -fno-strict-aliasing is used > > This warning is inadvertently triggered when linking BASE libraries built > with -mstrict-align into modules of types other than SEC or PEI, since the > types are subtly different, even though the use of code that maintains > strict alignment in a module that does not care about this is unlikely to > cause problems. And even if it did, it would still only affect code built > with -fstrict-aliasing enabled, which we disable unconditionally. So let's > just silence the warning by passing -Wno-lto-type-mismatch. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > 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 7a58ce365ed2..9a3ba9defb12 100755 > --- a/BaseTools/Conf/tools_def.template > +++ b/BaseTools/Conf/tools_def.template > @@ -5407,7 +5407,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS > DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 > > RELEASE_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -mcmodel=tiny -fomit-frame-pointer > -RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 > +RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch > > NOOPT_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small > NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0 > -- > 2.7.4 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Reviewed-by: Liming Gao <liming.gao@intel.com> >-----Original Message----- >From: Leif Lindholm [mailto:leif.lindholm@linaro.org] >Sent: Wednesday, June 21, 2017 4:00 AM >To: Ard Biesheuvel <ard.biesheuvel@linaro.org> >Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Zhu, >Yonghong <yonghong.zhu@intel.com>; lersek@redhat.com >Subject: Re: [PATCH 2/2] BaseTools/tools_def: AARCH64: disable LTO type >mismatch warnings > >On Tue, Jun 20, 2017 at 08:43:54PM +0200, Ard Biesheuvel wrote: >> On AARCH64, any code that may execute with the MMU off needs to be >built >> with -mstrict-align, given that unaligned accesses are not allowed unless >> the MMU is enabled. This does not only affect SEC and PEI modules, but >> also static libraries of the BASE type, which may be linked into such >> modules, as well as into modules of other types. As it turns out, the >> presence of -mstrict-align is reflected in the internal representations >> of the types defined in those libraries. >> >> When -fstrict-aliasing is passed to GCC, it assumes that pointers to >> objects of different types cannot refer to the same memory location, and >> attempts to exploit this fact when optimizing the code. Since such >> assumptions are only valid under very strict conditions which are not >> guaranteed to be met in EDK2, we disable this optimization by passing >> -fno-strict-aliasing by default. > >Just a comment - you don't necessarily have to update this excellent >commit mesage, but: >-fstrict-aliasing is GCC default, because it is a restriction in the C >language. Because it's a bit non-obvious, things can go hilariously >wrong in very non-obvious ways, and the potential optimization gains >are ulikely to be generally relevant, -fno-strict-aliasing is a >sensible thing to always have set (like we do). > >> When LTO is in effect, this applies equally to the code generation that >> may occur at link time, which is why the linker warns about unexpected >> differences in type definitions between the intermediate representations >> that are present in the object files being linked. This may result in >> warnings such as the one below, even if -fno-strict-aliasing is used: >> >> MdePkg/Include/Library/BaseLib.h:1712:1: >> warning: type of 'StrToGuid' does not match original declaration >> [-Wlto-type-mismatch] >> StrToGuid ( >> ^ >> MdePkg/Library/BaseLib/SafeString.c:1506:1: >> note: 'StrToGuid' was previously declared here >> StrToGuid ( >> ^ >> MdePkg/Library/BaseLib/SafeString.c:1506:1: >> note: code may be misoptimized unless -fno-strict-aliasing is used >> >> This warning is inadvertently triggered when linking BASE libraries built >> with -mstrict-align into modules of types other than SEC or PEI, since the >> types are subtly different, even though the use of code that maintains >> strict alignment in a module that does not care about this is unlikely to >> cause problems. And even if it did, it would still only affect code built >> with -fstrict-aliasing enabled, which we disable unconditionally. So let's >> just silence the warning by passing -Wno-lto-type-mismatch. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> > >> --- >> 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 7a58ce365ed2..9a3ba9defb12 100755 >> --- a/BaseTools/Conf/tools_def.template >> +++ b/BaseTools/Conf/tools_def.template >> @@ -5407,7 +5407,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS = >DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS >> DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 >> >> RELEASE_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) - >flto -Wno-unused-but-set-variable -mcmodel=tiny -fomit-frame-pointer >> -RELEASE_GCC5_AARCH64_DLINK_FLAGS = >DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os - >L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass- >through=-llto-aarch64 >> +RELEASE_GCC5_AARCH64_DLINK_FLAGS = >DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os - >L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass- >through=-llto-aarch64 -Wno-lto-type-mismatch >> >> NOOPT_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) - >O0 -mcmodel=small >> NOOPT_GCC5_AARCH64_DLINK_FLAGS = >DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0 >> -- >> 2.7.4 >> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 21 June 2017 at 02:11, Gao, Liming <liming.gao@intel.com> wrote: > Reviewed-by: Liming Gao <liming.gao@intel.com> > Thanks, pushed as 9ba8baae15ca >>-----Original Message----- >>From: Leif Lindholm [mailto:leif.lindholm@linaro.org] >>Sent: Wednesday, June 21, 2017 4:00 AM >>To: Ard Biesheuvel <ard.biesheuvel@linaro.org> >>Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Zhu, >>Yonghong <yonghong.zhu@intel.com>; lersek@redhat.com >>Subject: Re: [PATCH 2/2] BaseTools/tools_def: AARCH64: disable LTO type >>mismatch warnings >> >>On Tue, Jun 20, 2017 at 08:43:54PM +0200, Ard Biesheuvel wrote: >>> On AARCH64, any code that may execute with the MMU off needs to be >>built >>> with -mstrict-align, given that unaligned accesses are not allowed unless >>> the MMU is enabled. This does not only affect SEC and PEI modules, but >>> also static libraries of the BASE type, which may be linked into such >>> modules, as well as into modules of other types. As it turns out, the >>> presence of -mstrict-align is reflected in the internal representations >>> of the types defined in those libraries. >>> >>> When -fstrict-aliasing is passed to GCC, it assumes that pointers to >>> objects of different types cannot refer to the same memory location, and >>> attempts to exploit this fact when optimizing the code. Since such >>> assumptions are only valid under very strict conditions which are not >>> guaranteed to be met in EDK2, we disable this optimization by passing >>> -fno-strict-aliasing by default. >> >>Just a comment - you don't necessarily have to update this excellent >>commit mesage, but: >>-fstrict-aliasing is GCC default, because it is a restriction in the C >>language. Because it's a bit non-obvious, things can go hilariously >>wrong in very non-obvious ways, and the potential optimization gains >>are ulikely to be generally relevant, -fno-strict-aliasing is a >>sensible thing to always have set (like we do). >> >>> When LTO is in effect, this applies equally to the code generation that >>> may occur at link time, which is why the linker warns about unexpected >>> differences in type definitions between the intermediate representations >>> that are present in the object files being linked. This may result in >>> warnings such as the one below, even if -fno-strict-aliasing is used: >>> >>> MdePkg/Include/Library/BaseLib.h:1712:1: >>> warning: type of 'StrToGuid' does not match original declaration >>> [-Wlto-type-mismatch] >>> StrToGuid ( >>> ^ >>> MdePkg/Library/BaseLib/SafeString.c:1506:1: >>> note: 'StrToGuid' was previously declared here >>> StrToGuid ( >>> ^ >>> MdePkg/Library/BaseLib/SafeString.c:1506:1: >>> note: code may be misoptimized unless -fno-strict-aliasing is used >>> >>> This warning is inadvertently triggered when linking BASE libraries built >>> with -mstrict-align into modules of types other than SEC or PEI, since the >>> types are subtly different, even though the use of code that maintains >>> strict alignment in a module that does not care about this is unlikely to >>> cause problems. And even if it did, it would still only affect code built >>> with -fstrict-aliasing enabled, which we disable unconditionally. So let's >>> just silence the warning by passing -Wno-lto-type-mismatch. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >>Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org> >> >>> --- >>> 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 7a58ce365ed2..9a3ba9defb12 100755 >>> --- a/BaseTools/Conf/tools_def.template >>> +++ b/BaseTools/Conf/tools_def.template >>> @@ -5407,7 +5407,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS = >>DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS >>> DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 >>> >>> RELEASE_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) - >>flto -Wno-unused-but-set-variable -mcmodel=tiny -fomit-frame-pointer >>> -RELEASE_GCC5_AARCH64_DLINK_FLAGS = >>DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os - >>L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass- >>through=-llto-aarch64 >>> +RELEASE_GCC5_AARCH64_DLINK_FLAGS = >>DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os - >>L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass- >>through=-llto-aarch64 -Wno-lto-type-mismatch >>> >>> NOOPT_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) - >>O0 -mcmodel=small >>> NOOPT_GCC5_AARCH64_DLINK_FLAGS = >>DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0 >>> -- >>> 2.7.4 >>> _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template index 7a58ce365ed2..9a3ba9defb12 100755 --- a/BaseTools/Conf/tools_def.template +++ b/BaseTools/Conf/tools_def.template @@ -5407,7 +5407,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20 RELEASE_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) -flto -Wno-unused-but-set-variable -mcmodel=tiny -fomit-frame-pointer -RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 +RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch NOOPT_GCC5_AARCH64_CC_FLAGS = DEF(GCC5_AARCH64_CC_FLAGS) -O0 -mcmodel=small NOOPT_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0
On AARCH64, any code that may execute with the MMU off needs to be built with -mstrict-align, given that unaligned accesses are not allowed unless the MMU is enabled. This does not only affect SEC and PEI modules, but also static libraries of the BASE type, which may be linked into such modules, as well as into modules of other types. As it turns out, the presence of -mstrict-align is reflected in the internal representations of the types defined in those libraries. When -fstrict-aliasing is passed to GCC, it assumes that pointers to objects of different types cannot refer to the same memory location, and attempts to exploit this fact when optimizing the code. Since such assumptions are only valid under very strict conditions which are not guaranteed to be met in EDK2, we disable this optimization by passing -fno-strict-aliasing by default. When LTO is in effect, this applies equally to the code generation that may occur at link time, which is why the linker warns about unexpected differences in type definitions between the intermediate representations that are present in the object files being linked. This may result in warnings such as the one below, even if -fno-strict-aliasing is used: MdePkg/Include/Library/BaseLib.h:1712:1: warning: type of 'StrToGuid' does not match original declaration [-Wlto-type-mismatch] StrToGuid ( ^ MdePkg/Library/BaseLib/SafeString.c:1506:1: note: 'StrToGuid' was previously declared here StrToGuid ( ^ MdePkg/Library/BaseLib/SafeString.c:1506:1: note: code may be misoptimized unless -fno-strict-aliasing is used This warning is inadvertently triggered when linking BASE libraries built with -mstrict-align into modules of types other than SEC or PEI, since the types are subtly different, even though the use of code that maintains strict alignment in a module that does not care about this is unlikely to cause problems. And even if it did, it would still only affect code built with -fstrict-aliasing enabled, which we disable unconditionally. So let's just silence the warning by passing -Wno-lto-type-mismatch. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- BaseTools/Conf/tools_def.template | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel