Re: [edk2-devel] [Patch v3 10/11] EmulatorPkg: Enable CLANG9 tool chain

["Liming Gao" <liming.gao@intel.com>]

Andrew and Leif:
  Thanks for your comment. CLANG9 is different from CLANG38. And, CLANG9 will support LLVM9 or the above. 
  So, this tool chain should have the specific word for its purpose, and remove the version 9 to avoid the confuse. 

  I prefer the tool chain name as CLANGPE or CLANGPDB. CLANGPE may be better. Now, CLANG9 tool chain 
  build rule family just uses CLANGPE. CLANGPE lets user know this tool chain generates PE/COFF image. 
  And, PE/COFF image debug symbol is PDB. So, PE also means PDB. 

  Last, I just review the changes. Tool chain name update is not big. If you accept CLANGPE as tool chain name, 
  I will send the patch soon to catch 201911 stable tag. 

Thanks
Liming
>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Leif
>Lindholm
>Sent: Friday, November 08, 2019 2:37 AM
>To: Andrew Fish <afish@apple.com>
>Cc: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Ni, Ray
><ray.ni@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
>Subject: Re: [edk2-devel] [Patch v3 10/11] EmulatorPkg: Enable CLANG9 tool
>chain
>
>Hi Andrew,
>
>Yeah, I'm pretty easy with regards to what we change it to.
>Although if we're bikeshedding, I would prefer not using _ in the
>toolchain profile name.
>
>I'm ... optimistic ... it won't break any of my existing scripts, but
>since the build system uses it as a
><toolchain profile>_<architecture>_<build target> separator, it would
>be nice if we didn't risk confusion there.
>
>(I would be totally happy with PECLANG, CLANGPE, PDBCLANG, CLANGPDB or
>something along those lines.)
>
>Regards,
>
>Leif
>
>On Thu, Nov 07, 2019 at 11:54:04AM -0600, Andrew Fish wrote:
>> Leif,
>>
>> I think I proposed CLANG_PDB or CLANG_PECOFF. I seem to like
>CLANG_PDB as assuming the PDE debugging experience is awesome/exists
>on Linux and macOS is not a given.
>>
>> Thanks,
>>
>> Andrew Fish
>>
>> > On Nov 7, 2019, at 11:19 AM, Leif Lindholm <leif.lindholm@linaro.org>
>wrote:
>> >
>> > Oops, sorry, missed this in search.
>> >
>> > On Wed, Oct 30, 2019 at 03:43:44PM +0000, Liming Gao wrote:
>> >> Andrew:
>> >>
>> >>  I prefer to keep short CLANG9 as the tool chain name. I add wiki
>> >>  page
>> >>  https://github.com/tianocore/tianocore.github.io/wiki/CLANG9-Tools-
>Chain
>> >>  to introduce it.
>> >
>> > Why should users be expected to go and read documentation in order to
>> > learn that fundamental and incompatible changes to output and debug
>> > format has been made as part of what looks like a simple version bump?
>> >
>> >>  And, we have CLANG38 tool chain. It generates ELF
>> >>  image and DWARF debug symbol format. It can work with LLVM 9.0
>> >>  release. Current tool chain tag name includes compiler name and
>> >>  version. There is no specific info in the tool chain name. The
>> >>  developer can get the more tool chain information from wiki page.
>> >
>> > We aleady have the problem that people think they need to use GCC5 to
>> > build EDK2 since that is the highest named GCC toolchain profile.
>> >
>> > Let's not make this situation *worse* by setting up a multidimensional
>> > feature matrix, based off random numerical values.
>> >
>> > So say that we next have a pressing need to create a new toolchain
>> > profile for clang 10, using the old ELF mechanism. Are you then
>> > suggesting we set up a table on said wiki page where people have to go
>> > and look up what the object and debug formats are for each CLANG##
>> > toolchain profile?
>> >
>> > And what if you then end up needing to do the same for the PDB
>> > flavour?
>> >
>> >>  CLANG9 is designed to support Emulator for Windows host only.
>> >
>> > Which is why it makes no sense to name it as a successor of
>> > CLANG38.
>> >
>> >>  CLANG38 may be used for Emulator in Linux or Mac. I don’t
>> >>  try it before.
>> >>
>> >>  CLANG9 goal is to align the same compiler in the different host
>> >>  development environment. It can replace VS or GCC compiler. On
>> >>  Windows Host, I verify VS debugger for the source level debug. On
>> >>  Linux host, I have not verified llvm debugger. I will investigate
>> >>  the debugger solution for OVMF in Linux host.
>> >
>> > We are not asking you to throw out this toolchain profile.
>> >
>> > We are saying that since the functionality it provides is completely
>> > unrelated to that of CLANG38, it should not be named in a way that
>> > suggests it is merely a revision update.
>> >
>> > /
>> >    Leif
>> >
>> >> Thanks
>> >> Liming
>> >> From: afish@apple.com <afish@apple.com>
>> >> Sent: Saturday, October 26, 2019 2:45 AM
>> >> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
>> >> Cc: Ni, Ray <ray.ni@intel.com>; Justen, Jordan L
><jordan.l.justen@intel.com>
>> >> Subject: Re: [edk2-devel] [Patch v3 10/11] EmulatorPkg: Enable CLANG9
>tool chain
>> >>
>> >> Liming,
>> >>
>> >> Sorry I missed this mail. Thanks for the info! I was doing some research
>into this too and now I think I finally understand. I think the name for the tool
>chain really confused me and we should think about changing the name.
>> >>
>> >> From what I understand CLANG9 means produce PE/COFF directly and
>used the PDB debugging format. I see from the llvm site that the linker can
>produce PDB directly as you mention. This all makes sense to me now as LLVM
>tries to make it easy to be a drop in replacement for VC++ or GCC. So this tool
>chain is designed to be able to cross build a "Windows App" on a Linux or
>macOS. It also looks like the llvm debugger, lldb, is lagging in its support for
>PDB based debugging.
>> >>
>> >>
>> >> Anyway I think Leif and I agree the toolchain name is very confusing. I'd
>rather see it called CLANG9_WIN or CLANG_PDB.
>> >>
>> >>
>> >> On Oct 18, 2019, at 7:27 AM, Liming Gao
><liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:
>> >>
>> >> Andrew:
>> >> Here is the cover letter on CLANG9 introduction.
>https://edk2.groups.io/g/devel/message/49157
>> >>
>> >> 1)      Yes. CLANG9 tool chain is added to directly generate PE/COFF image
>(EFI image).
>> >> This tool chain uses LLVM clang C compiler and lld linker, generates
>PE/COFF
>> >> image and PDB compatible debug symbol format. Now, it supports
>IA32/X64 Archs.
>> >> LLVM clang C compiler and lld linker are the standalone tool set. They
>don’t depend other lib to generate PE/COFF image.
>> >>
>> >> 2)      Yes. CLANG9 is the cross OS tool chain. It can work on
>Windows/Linux/Mac host OS.
>> >> LLVM LLD linker uses Windows style arguments. I verify CLANG9 for
>Ovmf3264 in Windows/Linux host OS.
>> >>
>> >>
>> >> On Linux can you source level debug Ovmf?
>> >>
>> >>
>> >> 3)     This patch enables WinHost in Windows. It doesn’t enable UnixHost.
>Now, EmulatorPkg with CLANG9 only works on Windows Host.
>> >> This patch can make other modules pass build in Windows/Linux/Mac
>only if LLVM9 tool set is installed.
>> >> But, the generated image may not work on Linux/Mac. I agree below
>linker flags are specific to windows host.
>> >> So, I suggest to add the conditional statement of $(WIN_HOST_BUILD)
>== TRUE for them.
>> >>
>> >>
>> >> For the EmulatorPkg the Host is a native App for that OS you build on, but
>it seems like CLANG9 is targeted to build Windows Apps. I'm not sure  but you
>might be able to override all the linker commands to build a native app, or just
>use the system linker for the Host?
>> >>
>> >> I'm not sure how well debugging will work mixing PDB and DWARF
>symbol formats?
>> >>
>> >> Thanks,
>> >>
>> >> Andrew Fish
>> >>
>> >>
>> >>
>> >> !if $(WIN_HOST_BUILD) == TRUE
>> >>  GCC:*_CLANG9_*_DLINK_FLAGS     = /ALIGN:4096 /FILEALIGN:4096
>/SUBSYSTEM:CONSOLE
>> >>  GCC:DEBUG_CLANG9_*_DLINK_FLAGS =
>/EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
>> >>  GCC:NOOPT_CLANG9_*_DLINK_FLAGS =
>/EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
>> >> !endif
>> >>
>> >> Thanks
>> >> Liming
>> >> From: afish@apple.com<mailto:afish@apple.com>
><afish@apple.com<mailto:afish@apple.com>>
>> >> Sent: Friday, October 18, 2019 1:15 AM
>> >> To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
>> >> Cc: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>;
>devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Justen, Jordan L
><jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
>> >> Subject: Re: [Patch v3 10/11] EmulatorPkg: Enable CLANG9 tool chain
>> >>
>> >> Ray,
>> >>
>> >> Sorry I'm coming a little late to this and I'm confused. I have some
>questions?
>> >> 1) Does CLANG9 imply CLANGPE?
>> >> 2) Does CLANGPE work on Linux and macOS? Can you pass the Windows
>style arguments to CLANGPE linker on Linux and macOS?
>> >> 3) For the EmulatorPkg don't you have the extra requirement that
>compiler needs a standard C lib (or platform specific libs) to function?
>> >>       a) GCC:*_CLANG9_*_DLINK_FLAGS in EmulatorPkg.dsc seems to
>imply the Linux and macOS systems will have a Windows SKD dir and a lot of
>Windows DLLs? Does all that come when you install CLANG9 when you install
>it on Linux or macOS?
>> >>
>> >>
>> >>
>> >> So I guess I'm asking is the linker really the same for CLANG9 on all
>systems? I guess the answer could be yes, but it seems the C lib for the Host
>App is still an App for that OS and is OS dependent?
>> >>
>> >>
>> >>
>> >> Sorry if I'm missing something fundamental and this is a dumb question.
>> >>
>> >>
>> >>
>> >> Thanks,
>> >>
>> >>
>> >>
>> >> Andrew Fish
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >> On Oct 17, 2019, at 12:27 AM, Ni, Ray
><ray.ni@intel.com<mailto:ray.ni@intel.com>> wrote:
>> >>
>> >> Liming,
>> >> Emulator is using a generic SEC module. The host specific module is called
>"Host".
>> >> So I prefer to change the macro to "WIN_HOST_BUILD", with this change,
>Reviewed-by: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
>> >>
>> >>
>> >>
>> >> -----Original Message-----
>> >> From: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>
>> >> Sent: Thursday, October 17, 2019 2:56 PM
>> >> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
>> >> Cc: Justen, Jordan L
><jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Andrew
>Fish
>> >> <afish@apple.com<mailto:afish@apple.com>>; Ni, Ray
><ray.ni@intel.com<mailto:ray.ni@intel.com>>
>> >> Subject: [Patch v3 10/11] EmulatorPkg: Enable CLANG9 tool chain
>> >>
>> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1603
>> >> 1. Add WIN_SEC_BUILD macro check for CLANG9 tool chain build -p
>> >> EmulatorPkg\EmulatorPkg.dsc -a IA32 -DWIN_SEC_BUILD=TRUE -t
>CLANG9
>> >> build -p EmulatorPkg\EmulatorPkg.dsc -a X64 -DWIN_SEC_BUILD=TRUE -t
>> >> CLANG9 2. Append CLANG CC and LINK flags to generate windows HOST.
>> >> 3. Fix WinHost issue to call GetProcessAffinityMask() API.
>> >>  The input parameter should be UINTN pointer instead of UINT32 pointer.
>> >>
>> >> Cc: Jordan Justen
><jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
>> >> Cc: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
>> >> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
>> >> Signed-off-by: Liming Gao
><liming.gao@intel.com<mailto:liming.gao@intel.com>>
>> >> ---
>> >> EmulatorPkg/Win/Host/WinHost.c   | 6 +++---
>> >> EmulatorPkg/EmulatorPkg.dsc      | 7 ++++++-
>> >> EmulatorPkg/Win/Host/WinHost.inf | 6 ++++++
>> >> 3 files changed, 15 insertions(+), 4 deletions(-)
>> >>
>> >> diff --git a/EmulatorPkg/Win/Host/WinHost.c
>> >> b/EmulatorPkg/Win/Host/WinHost.c index 9aba3c8959..e40ce32548
>100644
>> >> --- a/EmulatorPkg/Win/Host/WinHost.c
>> >> +++ b/EmulatorPkg/Win/Host/WinHost.c
>> >> @@ -356,7 +356,7 @@ Returns:
>> >> INTN
>> >> EFIAPI
>> >> main (
>> >> -  IN  INTN  Argc,
>> >> +  IN  INT  Argc,
>> >>  IN  CHAR8 **Argv,
>> >>  IN  CHAR8 **Envp
>> >>  )
>> >> @@ -391,8 +391,8 @@ Returns:
>> >>  VOID                  *SecFile;
>> >>  CHAR16                *MemorySizeStr;
>> >>  CHAR16                *FirmwareVolumesStr;
>> >> -  UINT32                ProcessAffinityMask;
>> >> -  UINT32                SystemAffinityMask;
>> >> +  UINTN                 ProcessAffinityMask;
>> >> +  UINTN                 SystemAffinityMask;
>> >>  INT32                 LowBit;
>> >>
>> >>  //
>> >> diff --git a/EmulatorPkg/EmulatorPkg.dsc
>b/EmulatorPkg/EmulatorPkg.dsc
>> >> index 20f1187713..72532f5daf 100644
>> >> --- a/EmulatorPkg/EmulatorPkg.dsc
>> >> +++ b/EmulatorPkg/EmulatorPkg.dsc
>> >> @@ -237,9 +237,10 @@
>> >>
>> >> [Components]
>> >> !if "IA32" in $(ARCH) || "X64" in $(ARCH)
>> >> -  !if "MSFT" in $(FAMILY)
>> >> +  !if "MSFT" in $(FAMILY) || $(WIN_SEC_BUILD) == TRUE
>> >>    ##
>> >>    #  Emulator, OS WIN application
>> >> +    #  CLANG9 is cross OS tool chain. It depends on WIN_SEC_BUILD
>macro.
>> >>    ##
>> >>    EmulatorPkg/Win/Host/WinHost.inf
>> >>  !else
>> >> @@ -419,7 +420,11 @@
>> >>
>> >>  MSFT:DEBUG_*_*_CC_FLAGS = /Od /Oy-
>> >>  MSFT:NOOPT_*_*_CC_FLAGS = /Od /Oy-
>> >> +  GCC:DEBUG_CLANG9_*_CC_FLAGS =-O0 -Wno-unused-command-
>line-
>> >> argument
>> >> + -Wno-incompatible-pointer-types -Wno-enum-conversion
>> >> + -Wno-incompatible-pointer-types -Wno-sometimes-uninitialized
>> >> + -Wno-constant-conversion -Wno-main-return-type
>> >>
>> >>  MSFT:*_*_*_DLINK_FLAGS     = /ALIGN:4096 /FILEALIGN:4096
>> >> /SUBSYSTEM:CONSOLE
>> >>  MSFT:DEBUG_*_*_DLINK_FLAGS =
>> >> /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
>> >>  MSFT:NOOPT_*_*_DLINK_FLAGS =
>> >> /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
>> >> +  GCC:*_CLANG9_*_DLINK_FLAGS     = /ALIGN:4096 /FILEALIGN:4096
>> >> /SUBSYSTEM:CONSOLE
>> >> +  GCC:DEBUG_CLANG9_*_DLINK_FLAGS =
>> >> + /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
>> >> + GCC:NOOPT_CLANG9_*_DLINK_FLAGS =
>> >> + /EXPORT:InitializeDriver=$(IMAGE_ENTRY_POINT) /BASE:0x10000
>> >> diff --git a/EmulatorPkg/Win/Host/WinHost.inf
>> >> b/EmulatorPkg/Win/Host/WinHost.inf
>> >> index 631d5a6470..1adca10d79 100644
>> >> --- a/EmulatorPkg/Win/Host/WinHost.inf
>> >> +++ b/EmulatorPkg/Win/Host/WinHost.inf
>> >> @@ -95,3 +95,9 @@
>> >>  MSFT:*_VS2017_X64_DLINK_FLAGS      =
>> >> /LIBPATH:"%VCToolsInstallDir%lib\x64"
>> >> /LIBPATH:"%UniversalCRTSdkDir%lib\%UCRTVersion%\ucrt\x64"
>> >> /LIBPATH:"%WindowsSdkDir%lib\%WindowsSDKLibVersion%\um\x64"
>> >> /NOLOGO /SUBSYSTEM:CONSOLE /NODEFAULTLIB /IGNORE:4086 /MAP
>> >> /OPT:REF /DEBUG /MACHINE:AMD64 /LTCG Kernel32.lib MSVCRTD.lib
>> >> vcruntimed.lib ucrtd.lib Gdi32.lib User32.lib Winmm.lib Advapi32.lib
>> >>  MSFT:*_*_X64_ASM_FLAGS            == /nologo /W3 /WX /c /Cx /Zd /W0
>/Zi
>> >>  MSFT:*_*_X64_ASMLINK_FLAGS        == /link /nologo
>> >> +
>> >> +  GCC:*_CLANG9_X64_DLINK_FLAGS ==
>> >> /out:"$(BIN_DIR)\$(BASE_NAME).exe" /base:0x10000000
>> >> /pdb:"$(BIN_DIR)\$(BASE_NAME).pdb"
>> >> /LIBPATH:"%UniversalCRTSdkDir%lib\%UCRTVersion%\ucrt\x64"
>> >> /LIBPATH:"%WindowsSdkDir%lib\%WindowsSDKLibVersion%\um\x64"
>> >> /LIBPATH:"%VCToolsInstallDir%lib\x64"   /NOLOGO
>/SUBSYSTEM:CONSOLE
>> >> /NODEFAULTLIB /IGNORE:4086  /OPT:REF /DEBUG /MACHINE:AMD64
>> >> Kernel32.lib MSVCRTD.lib vcruntimed.lib ucrtd.lib Gdi32.lib User32.lib
>> >> Winmm.lib Advapi32.lib /lldmap
>> >> /EXPORT:InitializeDriver=_ModuleEntryPoint
>> >> +  GCC:*_CLANG9_X64_CC_FLAGS == -m64 -g -fshort-wchar
>> >> + -fno-strict-aliasing -Wall -c -include AutoGen.h -D
>> >> + _CRT_SECURE_NO_WARNINGS -Wnonportable-include-path  -D
>UNICODE
>> >> -D
>> >> + _CRT_SECURE_NO_DEPRECATE
>> >> +
>> >> +  GCC:*_CLANG9_IA32_DLINK_FLAGS ==
>> >> /out:"$(BIN_DIR)\$(BASE_NAME).exe" /base:0x10000000
>> >> /pdb:"$(BIN_DIR)\$(BASE_NAME).pdb"
>> >> /LIBPATH:"%UniversalCRTSdkDir%lib\%UCRTVersion%\ucrt\x86"
>> >> /LIBPATH:"%WindowsSdkDir%lib\%WindowsSDKLibVersion%\um\x86"
>> >> /LIBPATH:"%VCToolsInstallDir%ib\x86"   /NOLOGO
>/SUBSYSTEM:CONSOLE
>> >> /NODEFAULTLIB /IGNORE:4086  /OPT:REF /DEBUG /MACHINE:I386
>> >> Kernel32.lib MSVCRTD.lib vcruntimed.lib ucrtd.lib Gdi32.lib User32.lib
>> >> Winmm.lib Advapi32.lib /lldmap
>> >> /EXPORT:InitializeDriver=_ModuleEntryPoint
>> >> +  GCC:*_CLANG9_IA32_CC_FLAGS == -m32 -g -fshort-wchar
>> >> + -fno-strict-aliasing -Wall -c -include AutoGen.h -D
>> >> + _CRT_SECURE_NO_WARNINGS -Wnonportable-include-path  -D
>UNICODE
>> >> -D
>> >> + _CRT_SECURE_NO_DEPRECATE
>> >> --
>> >> 2.13.0.windows.1
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >
>> >
>> >
>>
>
>