Liming, Yes, perhaps that was unclear from my message. From what I understand __builtin_va_start behaves the same way as __builtin_ms_va__start for EFIAPI functions, and these are currently the only functions where variadic arguments are allowed.
For non-EFIAPI functions the calling conventions may be different from Microsoft 64-bit, i.e. they are System V for CLANG, CLANGPDB, and GCC. If you use variadic arguments in non-EFIAPI functions (which I believe is currently unsupported), you will end up with different calling conventions for variadic arguments as well. In this case the use of __builtin_va_start will work fine here, as System V variadic arguments will be used for non-EFIAPI, and Microsoft 64-bit for EFIAPI, while __builtin_ms_va_start may technically break non-EFIAPI. Perhaps I do not fully understand the underlying mechanism, but that is how I have always thought it works. Best wishes, Vitaly On Wed, Feb 12, 2020 at 17:01, Gao, Liming <liming....@intel.com> wrote: > Vitaly: > > With this change, X64 GCC and CLANG tool chain will use below VA_START > definition. > > #define VA_START(Marker, Parameter) __builtin_ms_va_start (Marker, Parameter) > > Thanks > > Liming > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly > Cheptsov via Groups.Io > Sent: Wednesday, February 12, 2020 4:08 PM > To: Liu, Zhiguang <zhiguang....@intel.com> > Cc: devel@edk2.groups.io; Gao, Liming <liming....@intel.com>; Shi, Steven > <steven....@intel.com> > Subject: Re: [edk2-devel] [PATCH 1/1] BaseTools: Switch to GNU mode for > CLANGPDB > > Liu, > > Thanks for explanation, it does make sense now. As for no need to > -DNO_MSABI_VA_FUNCS I agree, but it will not make much difference, because > from what I understand the VA_ARG implementation is chosen based on EFIAPI > presence when generic __builtin’s are used. > > Best, > > Vitaly > >> 12 февр. 2020 г., в 04:38, Liu, Zhiguang <zhiguang....@intel.com> написал(а): >> >> Hi Vitaly, >> >> After your patch to Switch to GNU mode for CLANGPDB, the build option >> -DNO_MSABI_VA_FUNCS is not required. I will send another patch to remove it. >> >> And for you question, this is a patch set that resolves BZ 2415, and the >> second patch 21821933aea284cd3dfea6994bd4b83bd9739fc9 has direct influence >> to CLANG38. >> >> Thanks >> >> Zhiguang >> >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly >> Cheptsov via Groups.Io >> Sent: Tuesday, February 11, 2020 3:09 PM >> To: Gao, Liming <liming....@intel.com>; Liu, Zhiguang >> <zhiguang....@intel.com>; Shi, Steven <steven....@intel.com> >> Cc: devel@edk2.groups.io >> Subject: Re: [edk2-devel] [PATCH 1/1] BaseTools: Switch to GNU mode for >> CLANGPDB >> >> Liming, >> >> Done. As a side note, I am not positive how can >> 7990438f1437f47990a8890dee51978cb8dbc25c[1] resolve BZ 2415[2]. The bug was >> about CLANG38, and the toolchain updated was CLANGPDB. While it makes sense >> to update CLANGPDB with this flag to stay clean (it will not make a >> difference for clang in GNU mode), CLANGPDB has nothing to do to CLANG38. >> >> Best wishes, >> >> Vitaly >> >> [1] https://bugzilla.tianocore.org/show_bug.cgi?id=2415 >> >> [2] >> https://github.com/tianocore/edk2/commit/7990438f1437f47990a8890dee51978cb8dbc25c >> >>> 11 февр. 2020 г., в 09:02, Gao, Liming <liming....@intel.com> написал(а): >>> >>> Vitaly: >>> >>> Can you update this patch based on the latest edk2 trunk? I will catch it >>> for edk2 Q1 stable tag. >>> >>> Thanks >>> >>> Liming >>> >>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Liming Gao >>> Sent: Tuesday, February 11, 2020 1:34 PM >>> To: vit9696 <vit9...@protonmail.com>; devel@edk2.groups.io >>> Subject: Re: [edk2-devel] [PATCH 1/1] BaseTools: Switch to GNU mode for >>> CLANGPDB >>> >>> Reviewed-by: Liming Gao <liming....@intel.com> >>> >>> From: vit9696 <vit9...@protonmail.com> >>> Sent: Tuesday, February 11, 2020 3:23 AM >>> To: Gao, Liming <liming....@intel.com>; devel@edk2.groups.io >>> Subject: RE: [edk2-devel] [PATCH 1/1] BaseTools: Switch to GNU mode for >>> CLANGPDB >>> >>> Liming, >>> >>> We did run several of our projects based on EDK II in X64 mode, DEBUG, >>> RELEASE, NOOPT. Noticed no change from XCODE5. >>> >>> We also tried building several EDK builtin packages like CryptoPkg, MdePkg, >>> MdeModulePkg. >>> >>> Best wishes, >>> >>> Vitaly >>> >>> В пн, февр. 10, 2020 в 16:47, Gao, Liming <liming....@intel.com> пишет: >>> >>>> Vitaly: >>>> This change is good. Can you your test for it? I verify this patch for >>>> Ovmf platform on Windows. It can make ovmf pass build with CLANGPDB. >>>> >>>> Thanks >>>> Liming >>>>> -----Original Message----- >>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly >>>>> Cheptsov via Groups.Io >>>>> Sent: Monday, February 10, 2020 6:59 PM >>>>> To: devel@edk2.groups.io >>>>> Subject: [edk2-devel] [PATCH 1/1] BaseTools: Switch to GNU mode for >>>>> CLANGPDB >>>>> >>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2397 >>>>> >>>>> Signed-off-by: Vitaly Cheptsov <vit9...@protonmail.com> >>>>> --- >>>>> BaseTools/Conf/tools_def.template | 6 +++--- >>>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/BaseTools/Conf/tools_def.template >>>>> b/BaseTools/Conf/tools_def.template >>>>> index feee2bbf16..6bf6c5768e 100755 >>>>> --- a/BaseTools/Conf/tools_def.template >>>>> +++ b/BaseTools/Conf/tools_def.template >>>>> @@ -2755,11 +2755,11 @@ RELEASE_CLANG38_AARCH64_DLINK_FLAGS = >>>>> DEF(CLANG38_AARCH64_DLINK_FLAGS) -flto -Wl >>>>> DEFINE CLANGPDB_IA32_PREFIX = ENV(CLANG_BIN) >>>>> DEFINE CLANGPDB_X64_PREFIX = ENV(CLANG_BIN) >>>>> >>>>> -DEFINE CLANGPDB_IA32_TARGET = -target i686-unknown-windows >>>>> -DEFINE CLANGPDB_X64_TARGET = -target x86_64-unknown-windows >>>>> +DEFINE CLANGPDB_IA32_TARGET = -target i686-unknown-windows-gnu >>>>> +DEFINE CLANGPDB_X64_TARGET = -target x86_64-unknown-windows-gnu >>>>> >>>>> DEFINE CLANGPDB_WARNING_OVERRIDES = -Wno-parentheses-equality >>>>> -Wno-tautological-compare -Wno-tautological-constant-out- >>>>> of-range-compare -Wno-empty-body -Wno-unused-const-variable -Wno-varargs >>>>> -Wno-unknown-warning-option -Wno-microsoft-enum- >>>>> forward-reference >>>>> -DEFINE CLANGPDB_ALL_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) >>>>> DEF(CLANGPDB_WARNING_OVERRIDES) -fno-stack-protector - >>>>> mms-bitfields -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas >>>>> -Wno-incompatible-library-redeclaration -fno- >>>>> asynchronous-unwind-tables -mno-implicit-float >>>>> -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang - >>>>> funsigned-char -fno-ms-extensions -Wno-null-dereference >>>>> -fms-compatibility -mno-stack-arg-probe >>>>> +DEFINE CLANGPDB_ALL_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) >>>>> DEF(CLANGPDB_WARNING_OVERRIDES) -fno-stack-protector - >>>>> fno-asynchronous-unwind-tables -funsigned-char >>>>> -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -Wno- >>>>> address -Wno-shift-negative-value -Wno-unknown-pragmas >>>>> -Wno-incompatible-library-redeclaration -Wno-null-dereference -mno- >>>>> implicit-float -mms-bitfields -mno-stack-arg-probe -nostdlib -nostdlibinc >>>>> >>>>> ########################### >>>>> # CLANGPDB IA32 definitions >>>>> -- >>>>> 2.21.1 (Apple Git-122.3) >>>>> >>>>> >>>>> -=-=-=-=-=-= >>>>> [Groups.io](http://groups.io/) Links: You receive all messages sent to >>>>> this group. >>>>> >>>>> View/Reply Online (#54130): https://edk2.groups.io/g/devel/message/54130 >>>>> Mute This Topic: https://groups.io/mt/71134286/1759384 >>>>> Group Owner: devel+ow...@edk2.groups.io >>>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [liming....@intel.com] >>>>> -=-=-=-=-=-= > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54298): https://edk2.groups.io/g/devel/message/54298 Mute This Topic: https://groups.io/mt/71134286/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
