Hi Liming,
On 2017.11.14 16:03, Gao, Liming wrote:
In fact, I have sent one patch serial to add VS2017 tool chain for
IA32 and X64. https://lists.01.org/pipermail/edk2-devel/2017-October/016175.html
I missed that proposal, else I would have used it as my base. Thanks for
pointing it out.
In my patch, I disable warning 4701&4703, because they are also disabled in
VS2015.
Okay. Then I see no objections to disabling them.
I didn't see the /wd options for those in tools_def.template with
VS2015, so I thought these needed to be addressed.
But from your looking at your proposal, I realize that these are
disabled in ProcessorBind.h. Obviously, I'll update my ARM/AARCH64
proposal so that the warnings for these toolchains are disabled there as
well, instead of tools_def.template.
In tools_def.template, to align other VS tool chain, I use VS2017_BIN for 32bit,
VS2017_BINX64 for 64bit. Could you follow this rule to define VS2017_BINARM for
arm, VS2017_BINAARCH64 for AARCH64?
I will do that.
On VS2017, I notice it doesn't set VS150COMNTOOLS env by default.
So, I use vswhere.exe to detect VS2017 installation path. When we build source
code with VS tool chain, we open normal cmd instead of VS cmd.
I see. I've always been opening a VS command prompt before calling the
EDK setup script, so I built my proposal around that. But I agree that
it makes sense to be able to build from regular prompt, so I will try to
follow your lead.
My only concern is that it may be difficult to locate the relevant ARM
toolchains without %WindowsSdkVerBinPath% being properly defined.
In your proposal, you seem to be hardcoding WINSDK10_PREFIX to something
like "%ProgramFiles(x86)%\Windows Kits\10\bin\x86" but that won't work
with ARM/ARM64 as we will need to get the actual version of the defayult
SDK installed for VS2017 (e.g. "C:\Program Files (x86)\Windows
Kits\10\bin\10.0.16299.0\") as the ARM toolchains will not work otherwise.
For instance, on my VS2017 installation the "arm64\" under "Windows
Kits\10\bin\" does not contain the latest version of the ARM64 tools and
libraries, which is problematic as proper ARM64 compilation support was
only enabled with the latest update. Only the one under "Windows
Kits\10\bin\10.0.16299.0\" does.
All in all, rather than try to guess the SDK path, I think we should try
to locate and call "%ProgramFiles(x86)%\Microsoft Visual
Studio\2017\Community\Common7\Tools\vsdevcmd\core\winsdk.bat", to have
the VS environment provide us with the actual SDK version to use, as
determined by Microsoft.
In your patch on ARM support, <stdarg.h> is included in Base.h.
This is a windows header file. So, VS env must be setup to build source
code with ARM arch. Right?
That is correct. I had a quick look at removing that header, but didn't
see much harm in keeping it. However, now that I understand our
constraints better, I will make sure that header is not referenced.
I will also do the same for memset_ms.c and memcpy_ms.c, introduced for
ARM/ARM64 (In CompilerIntrinsicsLib), as it references <stddef.h> for
size_t.
Last, I suggest you base on my patch to add VS2017 ARM and AARCH64 arch.
> For your patches, I suggest you separate them per package.
I will follow both these suggestions and send a v2 when ready.
Regards,
/Pete
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
