Re: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD declaration in Library

2016-01-28 Thread Gao, Liming
Reviewed-by: Liming Gao 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Wednesday, January 27, 2016 5:19 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD 
declaration in Library

VOID* Patchable PCD in Library has the different declaration from the
one in Driver, this issue that will cause GCC LTO build failure.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/GenC.py | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
b/BaseTools/Source/Python/AutoGen/GenC.py
index 93be718..3f0dfd9 100644
--- a/BaseTools/Source/Python/AutoGen/GenC.py
+++ b/BaseTools/Source/Python/AutoGen/GenC.py
@@ -1,9 +1,9 @@
 ## @file
 # Routines for generating AutoGen.h and AutoGen.c
 #
-# Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution.  The full text of the license may be 
found at
 # http://opensource.org/licenses/bsd-license.php
 #
@@ -1097,11 +1097,12 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, Pcd):
 GetModeSizeName = '_PCD_GET_MODE_SIZE' + '_' + Pcd.TokenCName
 
 Type = ''
 Array = ''
 if Pcd.DatumType == 'VOID*':
-Type = '(VOID *)'
+if Pcd.DefaultValue[0]== '{':
+Type = '(VOID *)'
 Array = '[]'
 PcdItemType = Pcd.Type
 PcdExCNameList  = []
 if PcdItemType in gDynamicExPcd:
 PcdExTokenName = '_PCD_TOKEN_' + TokenSpaceGuidCName + '_' + 
Pcd.TokenCName
@@ -1159,11 +1160,19 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, Pcd):
 else:
 AutoGenH.Append('#define %s(Value)  LibPcdSet%s(%s, 
(Value))\n' % (SetModeName, DatumSizeLib, PcdTokenName))
 AutoGenH.Append('#define %s(Value)  LibPcdSet%sS(%s, 
(Value))\n' % (SetModeStatusName, DatumSizeLib, PcdTokenName))
 if PcdItemType == TAB_PCDS_PATCHABLE_IN_MODULE:
 PcdVariableName = '_gPcd_' + 
gItemTypeStringDatabase[TAB_PCDS_PATCHABLE_IN_MODULE] + '_' + TokenCName
-AutoGenH.Append('extern volatile %s _gPcd_BinaryPatch_%s%s;\n' 
%(DatumType, TokenCName, Array) )
+if DatumType == 'VOID*':
+ArraySize = int(Pcd.MaxDatumSize, 0)
+if Pcd.DefaultValue[0] == 'L':
+ArraySize = ArraySize / 2
+Array = '[%d]' % ArraySize
+DatumType = ['UINT8', 'UINT16'][Pcd.DefaultValue[0] == 'L']
+AutoGenH.Append('extern %s _gPcd_BinaryPatch_%s%s;\n' %(DatumType, 
TokenCName, Array))
+else:
+AutoGenH.Append('extern volatile  %s  %s%s;\n' % (DatumType, 
PcdVariableName, Array))
 AutoGenH.Append('#define %s  %s_gPcd_BinaryPatch_%s\n' %(GetModeName, 
Type, TokenCName))
 if Pcd.DatumType == 'VOID*':
 AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
LibPatchPcdSetPtrAndSize((VOID *)_gPcd_BinaryPatch_%s, 
&_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
(Buffer))\n' % (SetModeName, Pcd.TokenCName, Pcd.TokenCName, Pcd.TokenCName))
 AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
LibPatchPcdSetPtrAndSizeS((VOID *)_gPcd_BinaryPatch_%s, 
&_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
(Buffer))\n' % (SetModeStatusName, Pcd.TokenCName, Pcd.TokenCName, 
Pcd.TokenCName))
 else:
-- 
2.6.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD declaration in Library

2016-01-28 Thread Laszlo Ersek
On 01/28/16 03:38, Gao, Liming wrote:
> Laszlo:
>   Someone evaluates GCC LTO feature and detects this issue. Now, we have no 
> clear plan to add LTO support. 

Thanks Scott for the great summary, and Liming for the info.
Laszlo

> 
> Thanks
> Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Thursday, January 28, 2016 1:34 AM
> To: Zhu, Yonghong
> Cc: edk2-de...@ml01.01.org; Scott Duplichan
> Subject: Re: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD 
> declaration in Library
> 
> On 01/27/16 10:18, Yonghong Zhu wrote:
>> VOID* Patchable PCD in Library has the different declaration from the 
>> one in Driver, this issue that will cause GCC LTO build failure.
> 
> Wow, you managed to build edk2 modules with -flto? How? Is that going to be 
> added to BaseTools?
> 
> Thanks!
> Laszlo
> 
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Yonghong Zhu 
>> ---
>>  BaseTools/Source/Python/AutoGen/GenC.py | 15 ---
>>  1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
>> b/BaseTools/Source/Python/AutoGen/GenC.py
>> index 93be718..3f0dfd9 100644
>> --- a/BaseTools/Source/Python/AutoGen/GenC.py
>> +++ b/BaseTools/Source/Python/AutoGen/GenC.py
>> @@ -1,9 +1,9 @@
>>  ## @file
>>  # Routines for generating AutoGen.h and AutoGen.c  # -# Copyright (c) 
>> 2007 - 2015, Intel Corporation. All rights reserved.
>> +# Copyright (c) 2007 - 2016, Intel Corporation. All rights 
>> +reserved.
>>  # This program and the accompanying materials  # are licensed and 
>> made available under the terms and conditions of the BSD License  # 
>> which accompanies this distribution.  The full text of the license may 
>> be found at  # http://opensource.org/licenses/bsd-license.php
>>  #
>> @@ -1097,11 +1097,12 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, 
>> Pcd):
>>  GetModeSizeName = '_PCD_GET_MODE_SIZE' + '_' + Pcd.TokenCName
>>  
>>  Type = ''
>>  Array = ''
>>  if Pcd.DatumType == 'VOID*':
>> -Type = '(VOID *)'
>> +if Pcd.DefaultValue[0]== '{':
>> +Type = '(VOID *)'
>>  Array = '[]'
>>  PcdItemType = Pcd.Type
>>  PcdExCNameList  = []
>>  if PcdItemType in gDynamicExPcd:
>>  PcdExTokenName = '_PCD_TOKEN_' + TokenSpaceGuidCName + '_' + 
>> Pcd.TokenCName @@ -1159,11 +1160,19 @@ def CreateLibraryPcdCode(Info, 
>> AutoGenC, AutoGenH, Pcd):
>>  else:
>>  AutoGenH.Append('#define %s(Value)  LibPcdSet%s(%s, 
>> (Value))\n' % (SetModeName, DatumSizeLib, PcdTokenName))
>>  AutoGenH.Append('#define %s(Value)  LibPcdSet%sS(%s, 
>> (Value))\n' % (SetModeStatusName, DatumSizeLib, PcdTokenName))
>>  if PcdItemType == TAB_PCDS_PATCHABLE_IN_MODULE:
>>  PcdVariableName = '_gPcd_' + 
>> gItemTypeStringDatabase[TAB_PCDS_PATCHABLE_IN_MODULE] + '_' + TokenCName
>> -AutoGenH.Append('extern volatile %s _gPcd_BinaryPatch_%s%s;\n' 
>> %(DatumType, TokenCName, Array) )
>> +if DatumType == 'VOID*':
>> +ArraySize = int(Pcd.MaxDatumSize, 0)
>> +if Pcd.DefaultValue[0] == 'L':
>> +ArraySize = ArraySize / 2
>> +Array = '[%d]' % ArraySize
>> +DatumType = ['UINT8', 'UINT16'][Pcd.DefaultValue[0] == 'L']
>> +AutoGenH.Append('extern %s _gPcd_BinaryPatch_%s%s;\n' 
>> %(DatumType, TokenCName, Array))
>> +else:
>> +AutoGenH.Append('extern volatile  %s  %s%s;\n' % 
>> + (DatumType, PcdVariableName, Array))
>>  AutoGenH.Append('#define %s  %s_gPcd_BinaryPatch_%s\n' 
>> %(GetModeName, Type, TokenCName))
>>  if Pcd.DatumType == 'VOID*':
>>  AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
>> LibPatchPcdSetPtrAndSize((VOID *)_gPcd_BinaryPatch_%s, 
>> &_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
>> (Buffer))\n' % (SetModeName, Pcd.TokenCName, Pcd.TokenCName, Pcd.TokenCName))
>>  AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
>> LibPatchPcdSetPtrAndSizeS((VOID *)_gPcd_BinaryPatch_%s, 
>> &_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
>> (Buffer))\n' % (SetModeStatusName, Pcd.TokenCName, Pcd.TokenCName, 
>> Pcd.TokenCName))
>>  else:
>>
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD declaration in Library

2016-01-27 Thread Laszlo Ersek
On 01/28/16 00:10, Andrew Fish wrote:
> 
>> On Jan 27, 2016, at 9:33 AM, Laszlo Ersek  wrote:
>>
>> On 01/27/16 10:18, Yonghong Zhu wrote:
>>> VOID* Patchable PCD in Library has the different declaration from the
>>> one in Driver, this issue that will cause GCC LTO build failure.
>>
>> Wow, you managed to build edk2 modules with -flto? How? Is that going to
>> be added to BaseTools?
>>
> 
> Laszlo,
> 
> I can tell you what you need to do for Xcode.
> DLINK flag you add: -object_path_lto $(DEST_DIR_DEBUG)/$(BASE_NAME).lto
> This is only needed for source level debug. 
> 
> Adding -flto to the CC_FLAGS makes the compiler emit LLVM, and if you do this 
> the linker knows what to do. And you can do the reverse and set -fno-lto to 
> turn it off, as last flag wins. So for example you can turn it by adding to 
> the CC_FLAG in your platform DSC if you want to test. 
> 
> Note sure how it works with GCC. 

Thank you for these hints, although I've never tried clang.

I vaguely recall someone had looked into this for gcc (maybe Scott?
that's why I CC'd him), but I don't remember why it didn't work out
ultimately.

Ah, right, here it is:

http://thread.gmane.org/gmane.comp.bios.tianocore.devel/17382

... It looks complicated. I fail to distill a clear idea of why that
patch wasn't merged.

Thanks
Laszlo

> 
> Thanks,
> 
> Andrew Fish
> 
>> Thanks!
>> Laszlo
>>
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Yonghong Zhu 
>>> ---
>>> BaseTools/Source/Python/AutoGen/GenC.py | 15 ---
>>> 1 file changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
>>> b/BaseTools/Source/Python/AutoGen/GenC.py
>>> index 93be718..3f0dfd9 100644
>>> --- a/BaseTools/Source/Python/AutoGen/GenC.py
>>> +++ b/BaseTools/Source/Python/AutoGen/GenC.py
>>> @@ -1,9 +1,9 @@
>>> ## @file
>>> # Routines for generating AutoGen.h and AutoGen.c
>>> #
>>> -# Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
>>> +# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
>>> # This program and the accompanying materials
>>> # are licensed and made available under the terms and conditions of the BSD 
>>> License
>>> # which accompanies this distribution.  The full text of the license may be 
>>> found at
>>> # http://opensource.org/licenses/bsd-license.php
>>> #
>>> @@ -1097,11 +1097,12 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, 
>>> Pcd):
>>> GetModeSizeName = '_PCD_GET_MODE_SIZE' + '_' + Pcd.TokenCName
>>>
>>> Type = ''
>>> Array = ''
>>> if Pcd.DatumType == 'VOID*':
>>> -Type = '(VOID *)'
>>> +if Pcd.DefaultValue[0]== '{':
>>> +Type = '(VOID *)'
>>> Array = '[]'
>>> PcdItemType = Pcd.Type
>>> PcdExCNameList  = []
>>> if PcdItemType in gDynamicExPcd:
>>> PcdExTokenName = '_PCD_TOKEN_' + TokenSpaceGuidCName + '_' + 
>>> Pcd.TokenCName
>>> @@ -1159,11 +1160,19 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, 
>>> Pcd):
>>> else:
>>> AutoGenH.Append('#define %s(Value)  LibPcdSet%s(%s, 
>>> (Value))\n' % (SetModeName, DatumSizeLib, PcdTokenName))
>>> AutoGenH.Append('#define %s(Value)  LibPcdSet%sS(%s, 
>>> (Value))\n' % (SetModeStatusName, DatumSizeLib, PcdTokenName))
>>> if PcdItemType == TAB_PCDS_PATCHABLE_IN_MODULE:
>>> PcdVariableName = '_gPcd_' + 
>>> gItemTypeStringDatabase[TAB_PCDS_PATCHABLE_IN_MODULE] + '_' + TokenCName
>>> -AutoGenH.Append('extern volatile %s _gPcd_BinaryPatch_%s%s;\n' 
>>> %(DatumType, TokenCName, Array) )
>>> +if DatumType == 'VOID*':
>>> +ArraySize = int(Pcd.MaxDatumSize, 0)
>>> +if Pcd.DefaultValue[0] == 'L':
>>> +ArraySize = ArraySize / 2
>>> +Array = '[%d]' % ArraySize
>>> +DatumType = ['UINT8', 'UINT16'][Pcd.DefaultValue[0] == 'L']
>>> +AutoGenH.Append('extern %s _gPcd_BinaryPatch_%s%s;\n' 
>>> %(DatumType, TokenCName, Array))
>>> +else:
>>> +AutoGenH.Append('extern volatile  %s  %s%s;\n' % (DatumType, 
>>> PcdVariableName, Array))
>>> AutoGenH.Append('#define %s  %s_gPcd_BinaryPatch_%s\n' 
>>> %(GetModeName, Type, TokenCName))
>>> if Pcd.DatumType == 'VOID*':
>>> AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
>>> LibPatchPcdSetPtrAndSize((VOID *)_gPcd_BinaryPatch_%s, 
>>> &_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
>>> (Buffer))\n' % (SetModeName, Pcd.TokenCName, Pcd.TokenCName, 
>>> Pcd.TokenCName))
>>> AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
>>> LibPatchPcdSetPtrAndSizeS((VOID *)_gPcd_BinaryPatch_%s, 
>>> &_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
>>> (Buffer))\n' % (SetModeStatusName, Pcd.TokenCName, Pcd.TokenCName, 
>>> Pcd.TokenCName))
>>> else:
>>>
>>
>> 

Re: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD declaration in Library

2016-01-27 Thread Gao, Liming
Laszlo:
  Someone evaluates GCC LTO feature and detects this issue. Now, we have no 
clear plan to add LTO support. 

Thanks
Liming
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Thursday, January 28, 2016 1:34 AM
To: Zhu, Yonghong
Cc: edk2-de...@ml01.01.org; Scott Duplichan
Subject: Re: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD 
declaration in Library

On 01/27/16 10:18, Yonghong Zhu wrote:
> VOID* Patchable PCD in Library has the different declaration from the 
> one in Driver, this issue that will cause GCC LTO build failure.

Wow, you managed to build edk2 modules with -flto? How? Is that going to be 
added to BaseTools?

Thanks!
Laszlo

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/AutoGen/GenC.py | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
> b/BaseTools/Source/Python/AutoGen/GenC.py
> index 93be718..3f0dfd9 100644
> --- a/BaseTools/Source/Python/AutoGen/GenC.py
> +++ b/BaseTools/Source/Python/AutoGen/GenC.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # Routines for generating AutoGen.h and AutoGen.c  # -# Copyright (c) 
> 2007 - 2015, Intel Corporation. All rights reserved.
> +# Copyright (c) 2007 - 2016, Intel Corporation. All rights 
> +reserved.
>  # This program and the accompanying materials  # are licensed and 
> made available under the terms and conditions of the BSD License  # 
> which accompanies this distribution.  The full text of the license may 
> be found at  # http://opensource.org/licenses/bsd-license.php
>  #
> @@ -1097,11 +1097,12 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, 
> Pcd):
>  GetModeSizeName = '_PCD_GET_MODE_SIZE' + '_' + Pcd.TokenCName
>  
>  Type = ''
>  Array = ''
>  if Pcd.DatumType == 'VOID*':
> -Type = '(VOID *)'
> +if Pcd.DefaultValue[0]== '{':
> +Type = '(VOID *)'
>  Array = '[]'
>  PcdItemType = Pcd.Type
>  PcdExCNameList  = []
>  if PcdItemType in gDynamicExPcd:
>  PcdExTokenName = '_PCD_TOKEN_' + TokenSpaceGuidCName + '_' + 
> Pcd.TokenCName @@ -1159,11 +1160,19 @@ def CreateLibraryPcdCode(Info, 
> AutoGenC, AutoGenH, Pcd):
>  else:
>  AutoGenH.Append('#define %s(Value)  LibPcdSet%s(%s, 
> (Value))\n' % (SetModeName, DatumSizeLib, PcdTokenName))
>  AutoGenH.Append('#define %s(Value)  LibPcdSet%sS(%s, 
> (Value))\n' % (SetModeStatusName, DatumSizeLib, PcdTokenName))
>  if PcdItemType == TAB_PCDS_PATCHABLE_IN_MODULE:
>  PcdVariableName = '_gPcd_' + 
> gItemTypeStringDatabase[TAB_PCDS_PATCHABLE_IN_MODULE] + '_' + TokenCName
> -AutoGenH.Append('extern volatile %s _gPcd_BinaryPatch_%s%s;\n' 
> %(DatumType, TokenCName, Array) )
> +if DatumType == 'VOID*':
> +ArraySize = int(Pcd.MaxDatumSize, 0)
> +if Pcd.DefaultValue[0] == 'L':
> +ArraySize = ArraySize / 2
> +Array = '[%d]' % ArraySize
> +DatumType = ['UINT8', 'UINT16'][Pcd.DefaultValue[0] == 'L']
> +AutoGenH.Append('extern %s _gPcd_BinaryPatch_%s%s;\n' 
> %(DatumType, TokenCName, Array))
> +else:
> +AutoGenH.Append('extern volatile  %s  %s%s;\n' % 
> + (DatumType, PcdVariableName, Array))
>  AutoGenH.Append('#define %s  %s_gPcd_BinaryPatch_%s\n' 
> %(GetModeName, Type, TokenCName))
>  if Pcd.DatumType == 'VOID*':
>  AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
> LibPatchPcdSetPtrAndSize((VOID *)_gPcd_BinaryPatch_%s, 
> &_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
> (Buffer))\n' % (SetModeName, Pcd.TokenCName, Pcd.TokenCName, Pcd.TokenCName))
>  AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
> LibPatchPcdSetPtrAndSizeS((VOID *)_gPcd_BinaryPatch_%s, 
> &_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
> (Buffer))\n' % (SetModeStatusName, Pcd.TokenCName, Pcd.TokenCName, 
> Pcd.TokenCName))
>  else:
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD declaration in Library

2016-01-27 Thread Scott Duplichan
Laszlo Ersek [mailto:ler...@redhat.com] wrote:

]Sent: Wednesday, January 27, 2016 05:57 PM
]To: Andrew Fish 
]Cc: Yonghong Zhu ; edk2-de...@ml01.01.org; Scott 
Duplichan 
]Subject: Re: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD 
declaration in Library
]
]On 01/28/16 00:10, Andrew Fish wrote:
]> 
]>> On Jan 27, 2016, at 9:33 AM, Laszlo Ersek  wrote:
]>>
]>> On 01/27/16 10:18, Yonghong Zhu wrote:
]>>> VOID* Patchable PCD in Library has the different declaration from the
]>>> one in Driver, this issue that will cause GCC LTO build failure.
]>>
]>> Wow, you managed to build edk2 modules with -flto? How? Is that going to
]>> be added to BaseTools?
]>>
]> 
]> Laszlo,
]> 
]> I can tell you what you need to do for Xcode.
]> DLINK flag you add: -object_path_lto $(DEST_DIR_DEBUG)/$(BASE_NAME).lto
]> This is only needed for source level debug. 
]> 
]> Adding -flto to the CC_FLAGS makes the compiler emit LLVM, and if you do 
this the linker knows what to do. ]And you can do the reverse and set -fno-lto 
to turn it off, as last flag wins. So for example you can turn ]it by adding to 
the CC_FLAG in your platform DSC if you want to test. 
]> 
]> Note sure how it works with GCC. 
]
]Thank you for these hints, although I've never tried clang.
]
]I vaguely recall someone had looked into this for gcc (maybe Scott?
]that's why I CC'd him), but I don't remember why it didn't work out
]ultimately.
]
]Ah, right, here it is:
]
]http://thread.gmane.org/gmane.comp.bios.tianocore.devel/17382
]
]... It looks complicated. I fail to distill a clear idea of why that
]patch wasn't merged.

I think the reason the patch was never merged is that it is indeed
complicated, and without someone really pushing for it, a complicated
change doesn't get committed. Until fairly recently, the dramatic 
code size reduction brought by -flto would justify the added complexity.
I think today the decline in flash chip prices has reduced the urgency
to reduce UEFI code size. For many applications, -flto can improve
performance. But because UEFI execution time is I/O bound in so many
cases, -flto isn't going to produce faster code. The one benefit I can
think of is boot time reduction due to reading a smaller FV from the
relatively slow flash chip.

Why is using -flto with UEFI complicated? Using -flto with application
code is easy. The reason is because UEFI does some unusual things. One
unusual thing UEFI does is use its own libraries for resolving compiler
generated helper function calls. To link -flto objects with libraries, a
special prefix on the linker command line (-Wl,-plugin-opt=-pass-through=)
is needed. For an application build, gcc invokes the linker and takes care
of this requirement by passing -Wl,-plugin-opt=-pass-through= along with
its own library path. Because UEFI uses its own helper function libs, it
is up to us to us to build the corresponding path to use with the pass
through option on the linker command line. The UEFI build tools have no
mechanism for doing this. The patches I made do not solve this problem,
but rather work around it by disabling -flto for code that calls these
functions. While gcc generates few helper function calls for X64, the
same is not true for ARM.

Even trying to disable -flto for a file presents a problem. Adding
GCC:*_*_*_CC_FLAGS = -fno-lto doesn't work because GCC44 fails when
an unknown flag is encountered.

Another concern is debugging. GCC docs still say:
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.
Because of this, there needs to be some ways to easily toggle the
gcc -flto feature for UEFI builds. Certainly NOOPT builds can omit
-flto. But I worry about forcing -flto for DEBUG or RELEASE builds.
I had used an environment variable to pass extra gcc flags so that
something like GCC_EXTRA_CC_FLAGS=-fno-lto could be used to
temporarily disable link time optimization. But that idea was
rejected.

Another consideration is the -flto requirement to pass compiler flags
to the link step. While this is doable without significant modification
to the UEFI build system, it does add complexity.

Use of -flto does trigger a few new warnings. Some seem to be caused
when a function from one compilation unit resolves a call from a
different compilation unit. For C code, it should be possible to link
even when the two compilation units use different function prototypes.
But use of -flto triggers a warning. While this is a useful warning
(or was it an error, I don't remember), I think it is incorrect. 

Those are the sorts of challenges I can remember. Some notes are here:
Skip over the first part about building gcc itself:
http://notabs.org/uefi/gcc-lto.htm

Thanks,
Scott





]Thanks
]Laszlo
]
]> 
]> Thanks,
]> 
]> Andrew Fish
]> 
]>> Thanks!
]>> Laszlo
]>>
]>>>
]>>> 

Re: [edk2] [Patch] BaseTools: Fix the bug for VOID* Patchable PCD declaration in Library

2016-01-27 Thread Laszlo Ersek
On 01/27/16 10:18, Yonghong Zhu wrote:
> VOID* Patchable PCD in Library has the different declaration from the
> one in Driver, this issue that will cause GCC LTO build failure.

Wow, you managed to build edk2 modules with -flto? How? Is that going to
be added to BaseTools?

Thanks!
Laszlo

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/AutoGen/GenC.py | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/GenC.py 
> b/BaseTools/Source/Python/AutoGen/GenC.py
> index 93be718..3f0dfd9 100644
> --- a/BaseTools/Source/Python/AutoGen/GenC.py
> +++ b/BaseTools/Source/Python/AutoGen/GenC.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # Routines for generating AutoGen.h and AutoGen.c
>  #
> -# Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
> +# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
>  # This program and the accompanying materials
>  # are licensed and made available under the terms and conditions of the BSD 
> License
>  # which accompanies this distribution.  The full text of the license may be 
> found at
>  # http://opensource.org/licenses/bsd-license.php
>  #
> @@ -1097,11 +1097,12 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, 
> Pcd):
>  GetModeSizeName = '_PCD_GET_MODE_SIZE' + '_' + Pcd.TokenCName
>  
>  Type = ''
>  Array = ''
>  if Pcd.DatumType == 'VOID*':
> -Type = '(VOID *)'
> +if Pcd.DefaultValue[0]== '{':
> +Type = '(VOID *)'
>  Array = '[]'
>  PcdItemType = Pcd.Type
>  PcdExCNameList  = []
>  if PcdItemType in gDynamicExPcd:
>  PcdExTokenName = '_PCD_TOKEN_' + TokenSpaceGuidCName + '_' + 
> Pcd.TokenCName
> @@ -1159,11 +1160,19 @@ def CreateLibraryPcdCode(Info, AutoGenC, AutoGenH, 
> Pcd):
>  else:
>  AutoGenH.Append('#define %s(Value)  LibPcdSet%s(%s, 
> (Value))\n' % (SetModeName, DatumSizeLib, PcdTokenName))
>  AutoGenH.Append('#define %s(Value)  LibPcdSet%sS(%s, 
> (Value))\n' % (SetModeStatusName, DatumSizeLib, PcdTokenName))
>  if PcdItemType == TAB_PCDS_PATCHABLE_IN_MODULE:
>  PcdVariableName = '_gPcd_' + 
> gItemTypeStringDatabase[TAB_PCDS_PATCHABLE_IN_MODULE] + '_' + TokenCName
> -AutoGenH.Append('extern volatile %s _gPcd_BinaryPatch_%s%s;\n' 
> %(DatumType, TokenCName, Array) )
> +if DatumType == 'VOID*':
> +ArraySize = int(Pcd.MaxDatumSize, 0)
> +if Pcd.DefaultValue[0] == 'L':
> +ArraySize = ArraySize / 2
> +Array = '[%d]' % ArraySize
> +DatumType = ['UINT8', 'UINT16'][Pcd.DefaultValue[0] == 'L']
> +AutoGenH.Append('extern %s _gPcd_BinaryPatch_%s%s;\n' 
> %(DatumType, TokenCName, Array))
> +else:
> +AutoGenH.Append('extern volatile  %s  %s%s;\n' % (DatumType, 
> PcdVariableName, Array))
>  AutoGenH.Append('#define %s  %s_gPcd_BinaryPatch_%s\n' 
> %(GetModeName, Type, TokenCName))
>  if Pcd.DatumType == 'VOID*':
>  AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
> LibPatchPcdSetPtrAndSize((VOID *)_gPcd_BinaryPatch_%s, 
> &_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
> (Buffer))\n' % (SetModeName, Pcd.TokenCName, Pcd.TokenCName, Pcd.TokenCName))
>  AutoGenH.Append('#define %s(SizeOfBuffer, Buffer)  
> LibPatchPcdSetPtrAndSizeS((VOID *)_gPcd_BinaryPatch_%s, 
> &_gPcd_BinaryPatch_Size_%s, (UINTN)_PCD_PATCHABLE_%s_SIZE, (SizeOfBuffer), 
> (Buffer))\n' % (SetModeStatusName, Pcd.TokenCName, Pcd.TokenCName, 
> Pcd.TokenCName))
>  else:
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel