Re: [PATCH] MdeModulePkg: Fix GCC48/GCC49 build error

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

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.