.../Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf | 3 +++ 1 file changed, 3 insertions(+)
- Fix GCC48/GCC49 build error
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Bell Song <binx.song@intel.com>
---
.../Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf | 3 +++
1 file changed, 3 insertions(+)
diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf
index 578f97f..4c9aff5 100644
--- a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf
+++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf
@@ -54,3 +54,6 @@
DebugLib
BaseMemoryLib
ExtractGuidedSectionLib
+
+[BuildOptions]
+ GCC:*_*_*_CC_FLAGS = -fno-builtin
--
2.10.2.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
adding Ard
On 04/01/17 10:38, Song, BinX wrote:
> - Fix GCC48/GCC49 build error
>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Bell Song <binx.song@intel.com>
> ---
> .../Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf
> index 578f97f..4c9aff5 100644
> --- a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf
> +++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf
> @@ -54,3 +54,6 @@
> DebugLib
> BaseMemoryLib
> ExtractGuidedSectionLib
> +
> +[BuildOptions]
> + GCC:*_*_*_CC_FLAGS = -fno-builtin
>
In "BaseTools/Conf/tools_def.template", we currently have:
DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...]
DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...]
DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -fno-builtin
DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -fno-builtin
Now, GCC5_IA32_CC_FLAGS goes back to GCC44_ALL_CC_FLAGS, via:
- GCC49_IA32_CC_FLAGS
- GCC48_IA32_CC_FLAGS
- GCC47_IA32_CC_FLAGS
- GCC46_IA32_CC_FLAGS
- GCC45_IA32_CC_FLAGS
- GCC44_IA32_CC_FLAGS
- GCC44_ALL_CC_FLAGS
(similarly for GCC5_X64_CC_FLAGS.)
So, instead of this patch for BrotliCustomDecompressLib, how about:
- moving "-fno-builtin" from
GCC_ARM_CC_FLAGS and
GCC_AARCH64_CC_FLAGS
to
GCC_ALL_CC_FLAGS, and
- moving "-fno-builtin" from
GCC5_IA32_CC_FLAGS and
GCC5_X64_CC_FLAGS
to
GCC44_ALL_CC_FLAGS?
Do we have any reason for permitting builtins at all?
Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 3 April 2017 at 17:16, Laszlo Ersek <lersek@redhat.com> wrote: > adding Ard > > On 04/01/17 10:38, Song, BinX wrote: >> - Fix GCC48/GCC49 build error >> >> Cc: Liming Gao <liming.gao@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Bell Song <binx.song@intel.com> >> --- >> .../Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >> index 578f97f..4c9aff5 100644 >> --- a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >> +++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >> @@ -54,3 +54,6 @@ >> DebugLib >> BaseMemoryLib >> ExtractGuidedSectionLib >> + >> +[BuildOptions] >> + GCC:*_*_*_CC_FLAGS = -fno-builtin >> > > In "BaseTools/Conf/tools_def.template", we currently have: > > DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] > DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] > > DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -fno-builtin > DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -fno-builtin > > Now, GCC5_IA32_CC_FLAGS goes back to GCC44_ALL_CC_FLAGS, via: > - GCC49_IA32_CC_FLAGS > - GCC48_IA32_CC_FLAGS > - GCC47_IA32_CC_FLAGS > - GCC46_IA32_CC_FLAGS > - GCC45_IA32_CC_FLAGS > - GCC44_IA32_CC_FLAGS > - GCC44_ALL_CC_FLAGS > > (similarly for GCC5_X64_CC_FLAGS.) > > So, instead of this patch for BrotliCustomDecompressLib, how about: > > - moving "-fno-builtin" from > GCC_ARM_CC_FLAGS and > GCC_AARCH64_CC_FLAGS > to > GCC_ALL_CC_FLAGS, and > > - moving "-fno-builtin" from > GCC5_IA32_CC_FLAGS and > GCC5_X64_CC_FLAGS > to > GCC44_ALL_CC_FLAGS? > > Do we have any reason for permitting builtins at all? > Well, one thing I noticed the other day is that GCC does not 'recognize' memcpy() and memset() when -fno-builtin is defined, which means trivial memcpys and memsets will not be inlined. I guess that argues for not permitting it at all, but it also means adding it unconditionally may affect how code is currently generated for some platforms. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 04/03/17 18:21, Ard Biesheuvel wrote: > On 3 April 2017 at 17:16, Laszlo Ersek <lersek@redhat.com> wrote: >> adding Ard >> >> On 04/01/17 10:38, Song, BinX wrote: >>> - Fix GCC48/GCC49 build error >>> >>> Cc: Liming Gao <liming.gao@intel.com> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Bell Song <binx.song@intel.com> >>> --- >>> .../Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >>> index 578f97f..4c9aff5 100644 >>> --- a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >>> +++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >>> @@ -54,3 +54,6 @@ >>> DebugLib >>> BaseMemoryLib >>> ExtractGuidedSectionLib >>> + >>> +[BuildOptions] >>> + GCC:*_*_*_CC_FLAGS = -fno-builtin >>> >> >> In "BaseTools/Conf/tools_def.template", we currently have: >> >> DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] >> DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] >> >> DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -fno-builtin >> DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -fno-builtin >> >> Now, GCC5_IA32_CC_FLAGS goes back to GCC44_ALL_CC_FLAGS, via: >> - GCC49_IA32_CC_FLAGS >> - GCC48_IA32_CC_FLAGS >> - GCC47_IA32_CC_FLAGS >> - GCC46_IA32_CC_FLAGS >> - GCC45_IA32_CC_FLAGS >> - GCC44_IA32_CC_FLAGS >> - GCC44_ALL_CC_FLAGS >> >> (similarly for GCC5_X64_CC_FLAGS.) >> >> So, instead of this patch for BrotliCustomDecompressLib, how about: >> >> - moving "-fno-builtin" from >> GCC_ARM_CC_FLAGS and >> GCC_AARCH64_CC_FLAGS >> to >> GCC_ALL_CC_FLAGS, and >> >> - moving "-fno-builtin" from >> GCC5_IA32_CC_FLAGS and >> GCC5_X64_CC_FLAGS >> to >> GCC44_ALL_CC_FLAGS? >> >> Do we have any reason for permitting builtins at all? >> > > Well, one thing I noticed the other day is that GCC does not > 'recognize' memcpy() and memset() when -fno-builtin is defined, which > means trivial memcpys and memsets will not be inlined. But memcpy() and memset(), as written, cannot be called in edk2 anyway - explicitly, because we don't allow that, - implicitly (via struct assignment or initialization, for example), because we forbid that as well -- source code is supposed to use CopyMem(), CopyGuid(), and the like. So, yes, memcpy() and memset() would not be inlined with my suggestion, but edk2 code shouldn't exist in the first place that leads to the generation of memcpy() and memset() calls. > I guess that > argues for not permitting it at all, but it also means adding it > unconditionally may affect how code is currently generated for some > platforms. If said code doesn't conform to the above requirement, then yes, it could happen. Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
I agree to move this option to common GCC option. And, now GCC5 has this option. That means if the platform pass GCC5 build, it should not be impacted in GCC48/GCC49 by this change. Right? Thanks Liming > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Tuesday, April 4, 2017 12:34 AM > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Song, BinX <binx.song@intel.com>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com> > Subject: Re: [edk2] [PATCH] MdeModulePkg: Fix GCC48/GCC49 build error > > On 04/03/17 18:21, Ard Biesheuvel wrote: > > On 3 April 2017 at 17:16, Laszlo Ersek <lersek@redhat.com> wrote: > >> adding Ard > >> > >> On 04/01/17 10:38, Song, BinX wrote: > >>> - Fix GCC48/GCC49 build error > >>> > >>> Cc: Liming Gao <liming.gao@intel.com> > >>> Contributed-under: TianoCore Contribution Agreement 1.0 > >>> Signed-off-by: Bell Song <binx.song@intel.com> > >>> --- > >>> .../Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf > b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf > >>> index 578f97f..4c9aff5 100644 > >>> --- a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf > >>> +++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf > >>> @@ -54,3 +54,6 @@ > >>> DebugLib > >>> BaseMemoryLib > >>> ExtractGuidedSectionLib > >>> + > >>> +[BuildOptions] > >>> + GCC:*_*_*_CC_FLAGS = -fno-builtin > >>> > >> > >> In "BaseTools/Conf/tools_def.template", we currently have: > >> > >> DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] > >> DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] > >> > >> DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -fno-builtin > >> DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -fno-builtin > >> > >> Now, GCC5_IA32_CC_FLAGS goes back to GCC44_ALL_CC_FLAGS, via: > >> - GCC49_IA32_CC_FLAGS > >> - GCC48_IA32_CC_FLAGS > >> - GCC47_IA32_CC_FLAGS > >> - GCC46_IA32_CC_FLAGS > >> - GCC45_IA32_CC_FLAGS > >> - GCC44_IA32_CC_FLAGS > >> - GCC44_ALL_CC_FLAGS > >> > >> (similarly for GCC5_X64_CC_FLAGS.) > >> > >> So, instead of this patch for BrotliCustomDecompressLib, how about: > >> > >> - moving "-fno-builtin" from > >> GCC_ARM_CC_FLAGS and > >> GCC_AARCH64_CC_FLAGS > >> to > >> GCC_ALL_CC_FLAGS, and > >> > >> - moving "-fno-builtin" from > >> GCC5_IA32_CC_FLAGS and > >> GCC5_X64_CC_FLAGS > >> to > >> GCC44_ALL_CC_FLAGS? > >> > >> Do we have any reason for permitting builtins at all? > >> > > > > Well, one thing I noticed the other day is that GCC does not > > 'recognize' memcpy() and memset() when -fno-builtin is defined, which > > means trivial memcpys and memsets will not be inlined. > > But memcpy() and memset(), as written, cannot be called in edk2 anyway > > - explicitly, because we don't allow that, > > - implicitly (via struct assignment or initialization, for example), > because we forbid that as well -- source code is supposed to use > CopyMem(), CopyGuid(), and the like. > > So, yes, memcpy() and memset() would not be inlined with my suggestion, > but edk2 code shouldn't exist in the first place that leads to the > generation of memcpy() and memset() calls. > > > I guess that > > argues for not permitting it at all, but it also means adding it > > unconditionally may affect how code is currently generated for some > > platforms. > > If said code doesn't conform to the above requirement, then yes, it > could happen. > > Thanks! > Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 04/05/17 06:52, Gao, Liming wrote: > I agree to move this option to common GCC option. > > And, now GCC5 has this option. That means if the platform pass GCC5 > build, it should not be impacted in GCC48/GCC49 by this change. > Right? I think so, yes. Laszlo > > Thanks > Liming >> -----Original Message----- >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Tuesday, April 4, 2017 12:34 AM >> To: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Cc: Song, BinX <binx.song@intel.com>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com> >> Subject: Re: [edk2] [PATCH] MdeModulePkg: Fix GCC48/GCC49 build error >> >> On 04/03/17 18:21, Ard Biesheuvel wrote: >>> On 3 April 2017 at 17:16, Laszlo Ersek <lersek@redhat.com> wrote: >>>> adding Ard >>>> >>>> On 04/01/17 10:38, Song, BinX wrote: >>>>> - Fix GCC48/GCC49 build error >>>>> >>>>> Cc: Liming Gao <liming.gao@intel.com> >>>>> Contributed-under: TianoCore Contribution Agreement 1.0 >>>>> Signed-off-by: Bell Song <binx.song@intel.com> >>>>> --- >>>>> .../Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >> b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >>>>> index 578f97f..4c9aff5 100644 >>>>> --- a/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >>>>> +++ b/MdeModulePkg/Library/BrotliCustomDecompressLib/BrotliCustomDecompressLib.inf >>>>> @@ -54,3 +54,6 @@ >>>>> DebugLib >>>>> BaseMemoryLib >>>>> ExtractGuidedSectionLib >>>>> + >>>>> +[BuildOptions] >>>>> + GCC:*_*_*_CC_FLAGS = -fno-builtin >>>>> >>>> >>>> In "BaseTools/Conf/tools_def.template", we currently have: >>>> >>>> DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] >>>> DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) [...] -fno-builtin [...] >>>> >>>> DEFINE GCC5_IA32_CC_FLAGS = DEF(GCC49_IA32_CC_FLAGS) -fno-builtin >>>> DEFINE GCC5_X64_CC_FLAGS = DEF(GCC49_X64_CC_FLAGS) -fno-builtin >>>> >>>> Now, GCC5_IA32_CC_FLAGS goes back to GCC44_ALL_CC_FLAGS, via: >>>> - GCC49_IA32_CC_FLAGS >>>> - GCC48_IA32_CC_FLAGS >>>> - GCC47_IA32_CC_FLAGS >>>> - GCC46_IA32_CC_FLAGS >>>> - GCC45_IA32_CC_FLAGS >>>> - GCC44_IA32_CC_FLAGS >>>> - GCC44_ALL_CC_FLAGS >>>> >>>> (similarly for GCC5_X64_CC_FLAGS.) >>>> >>>> So, instead of this patch for BrotliCustomDecompressLib, how about: >>>> >>>> - moving "-fno-builtin" from >>>> GCC_ARM_CC_FLAGS and >>>> GCC_AARCH64_CC_FLAGS >>>> to >>>> GCC_ALL_CC_FLAGS, and >>>> >>>> - moving "-fno-builtin" from >>>> GCC5_IA32_CC_FLAGS and >>>> GCC5_X64_CC_FLAGS >>>> to >>>> GCC44_ALL_CC_FLAGS? >>>> >>>> Do we have any reason for permitting builtins at all? >>>> >>> >>> Well, one thing I noticed the other day is that GCC does not >>> 'recognize' memcpy() and memset() when -fno-builtin is defined, which >>> means trivial memcpys and memsets will not be inlined. >> >> But memcpy() and memset(), as written, cannot be called in edk2 anyway >> >> - explicitly, because we don't allow that, >> >> - implicitly (via struct assignment or initialization, for example), >> because we forbid that as well -- source code is supposed to use >> CopyMem(), CopyGuid(), and the like. >> >> So, yes, memcpy() and memset() would not be inlined with my suggestion, >> but edk2 code shouldn't exist in the first place that leads to the >> generation of memcpy() and memset() calls. >> >>> I guess that >>> argues for not permitting it at all, but it also means adding it >>> unconditionally may affect how code is currently generated for some >>> platforms. >> >> If said code doesn't conform to the above requirement, then yes, it >> could happen. >> >> Thanks! >> Laszlo > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.