Bug: PreferredToolArchitecture x64 should default, not override, conditionally for x86

Scottj1s commented 1 year ago

Version

No response

Summary

PreferredToolArchitecture is currently set to x64 to address out of memory issues due to large PCH files, when targeting x86. Using the x64 toolchain to cross compile for x86 allows much larger memory use and reduces these OOMs. This is a problem for ARM/ARM64 builds, causing underbuilds. These architectures should use their own native toolchains. PreferredToolArchitecture should be set to x64 for x86 only as a conditional default.

Reproducible example

No response

Expected behavior

No response

Actual behavior

No response

Additional comments

No response

sylveon commented 1 year ago

Wouldn't you want PreferredToolArchitecture to be x64 when targeting ARM64 if the host machine is x64? Also doesn't the same benefit apply if building ARM from ARM64, where PreferredToolArchitecture being set to ARM64 would let the compiler use more memory?

Scottj1s commented 1 year ago

possibly, though I'm not sure how one would determine machine arch in msbuild (ideas?) in any case, we could make the condition simply a check for PreferredToolArchitecture not being defined (versus also checking for x86 target arch), moving it the targets file to allow a project to define it upstream.

i.e. Microsoft.Windows.CppWinRT.Targets: <PreferredToolArchitecture Condition="'$(PreferredToolArchitecture)'==''">x64</PreferredToolArchitecture>

jlaanstra commented 1 year ago

You can use $(PROCESSOR_ARCHITECTURE)

Scottj1s commented 1 year ago

copy that, so:

Microsoft.Windows.CppWinRT.Targets: <PreferredToolArchitecture Condition="'$(PROCESSOR_ARCHITECTURE)'=='AMD64' and '$(PreferredToolArchitecture)'==''">x64</PreferredToolArchitecture>

jlaanstra commented 1 year ago

@Scottj1s Settings this in targets probably won't work, as the VC props files pick this up early on. Setting it in targets will be too late.

Scottj1s commented 1 year ago

Turns out we just shouldn't be taking this responsibility anyway, since VS 2022 does a better job.

andrew-rogers-wdc commented 10 months ago

Hi @Scottj1s,

This is the logic from Microsoft.Cpp.ToolsetLocation.props in the latest public VS2022 preview build (17.8.0 preview 5):

  <PropertyGroup>
    <!-- UseNativeEnvironment and _IsNativeEnvironment was used in prev versions to define if we want to use 64-bit tools when building for x64 platform. -->
    <PreferredToolArchitecture Condition="'$(PreferredToolArchitecture)' == '' and ('$(UseNativeEnvironment)' == 'true' or '$(_IsNativeEnvironment)' == 'true')">x64</PreferredToolArchitecture>

    <!-- By default we use the same bitness as the hosting platform -->
    <PreferredToolArchitecture Condition="'$(PreferredToolArchitecture)' == '' and ('$(PROCESSOR_ARCHITECTURE)' == 'AMD64' and '$(Platform)' == 'x64')">x64</PreferredToolArchitecture>
    <!-- prefer arm64 tools when on arm64 -->
    <PreferredToolArchitecture Condition="'$(PreferredToolArchitecture)' == '' and '$(PROCESSOR_ARCHITECTURE)' == 'ARM64' and '$(HostARM64Suported)' != 'false'">arm64</PreferredToolArchitecture>

    <PreferredToolArchitecture Condition="'$(PreferredToolArchitecture)' == ''">x86</PreferredToolArchitecture>

    <!-- If OS is not x64, we cannot use x64 tools even if preferred -->
    <PreferredToolArchitecture Condition="'$(PreferredToolArchitecture)' == 'x64' and ('$(PROCESSOR_ARCHITECTURE)' != 'AMD64' and '$(PROCESSOR_ARCHITEW6432)' != 'AMD64' and '$(PROCESSOR_ARCHITECTURE)' != 'ARM64')">x86</PreferredToolArchitecture>

    <!-- If OS is not arm64, we cannot use arm64 tools even if preferred -->
    <PreferredToolArchitecture Condition="'$(PreferredToolArchitecture)' == 'arm64' and '$(PROCESSOR_ARCHITECTURE)' != 'ARM64'">x86</PreferredToolArchitecture>
  </PropertyGroup>

Assume '$(PreferredToolArchitecture)' == '' before evaluating the properties in this PropertyGroup, and that a build targeting x86 is being performed:

UseNativeEnvironment and _IsNativeEnvironment are only applicable to builds targeting x64. Therefore the condition for this property:

   <!-- UseNativeEnvironment and _IsNativeEnvironment was used in prev versions to define if we want to use 64-bit tools when building for x64 platform. -->
   <PreferredToolArchitecture Condition="'$(PreferredToolArchitecture)' == '' and ('$(UseNativeEnvironment)' == 'true' or '$(_IsNativeEnvironment)' == 'true')">x64</PreferredToolArchitecture>

will not be satisfied for an x86 build (since $(UseNativeEnvironment)' == '' and '$(_IsNativeEnvironment)' == '').

The PreferredToolArchitecture property will not be set to x64 and will remain ''.

Similarly, since this is an x86 build, '$(Platform)' != 'x64' and the condition for this property will not be satisfied either:

   <!-- By default we use the same bitness as the hosting platform -->
   <PreferredToolArchitecture Condition="'$(PreferredToolArchitecture)' == '' and ('$(PROCESSOR_ARCHITECTURE)' == 'AMD64' and '$(Platform)' == 'x64')">x64</PreferredToolArchitecture>

The PreferredToolArchitecture property will, again, therefore not be set to x64 and will remain ''.

There are no further PreferredToolArchitecture properties with value x64. Therefore the PreferredToolArchitecture property will remain ''.

Taken together, this means the latest VS2022 will not automatically set PreferredToolArchitecture to x64, unless targeting x64 in a 64-bit msbuild, or if UseNativeEnvironment is set to 'true'.

Here is a worked example of a minimal C++ project which can be built with msbuild:

C:\scratch\console>type main.cpp
int main()
{
    return 0;
}

C:\scratch\console>type test.vcxproj
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <ItemGroup Label="ProjectConfigurations">
    <ProjectConfiguration Include="Debug|Win32" />
    <ProjectConfiguration Include="Debug|x64" />
  </ItemGroup>
  <PropertyGroup Label="Globals">
    <VCProjectVersion>17.0</VCProjectVersion>
    <ProjectGuid>{7fdbd260-5c80-49c1-a1e1-8ca1ef55e6a1}</ProjectGuid>
  </PropertyGroup>
  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
  <PropertyGroup Label="Configuration">
    <PlatformToolset>v143</PlatformToolset>
  </PropertyGroup>
  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
  <ItemGroup>
    <ClCompile Include="main.cpp" />
  </ItemGroup>
</Project>

Which then shows only a 64-bit msbuild targeting x64 will set PreferredToolArchitecture = 'x64':

C:\scratch\console>"c:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\amd64\MSBuild.exe" test.vcxproj /t:Rebuild /m /p:Configuration=Debug;Platform=Win32 -verbosity:diag | findstr PreferredToolArchitecture
                   PreferredToolArchitecture = x86

C:\scratch\console>"c:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\amd64\MSBuild.exe" test.vcxproj /t:Rebuild /m /p:Configuration=Debug;Platform=x64 -verbosity:diag | findstr PreferredToolArchitecture
                   PreferredToolArchitecture = x64

C:\scratch\console>"c:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\MSBuild.exe" test.vcxproj /t:Rebuild /m /p:Configuration=Debug;Platform=Win32 -verbosity:diag | findstr PreferredToolArchitecture
                   PreferredToolArchitecture = x86

C:\scratch\console>"c:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\MSBuild.exe" test.vcxproj /t:Rebuild /m /p:Configuration=Debug;Platform=x64 -verbosity:diag | findstr PreferredToolArchitecture
                   PreferredToolArchitecture = x86

We can also demonstrate how setting UseNativeEnvironment = true will cause PreferredToolArchitecture = x64 on a 64-bit OS, if we are in either a native:

C:\scratch\console>set PROCESSOR_ARCH
PROCESSOR_ARCHITECTURE=AMD64

C:\scratch\console>"c:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\MSBuild.exe" test.vcxproj /t:Rebuild /m /p:Configuration=Debug;Platform=Win32;UseNativeEnvironment=true -verbosity:diag | findstr PreferredToolArchitecture
                   PreferredToolArchitecture = x64

or Wow64 command prompt:

C:\scratch\console>set PROCESSOR_ARCH
PROCESSOR_ARCHITECTURE=x86
PROCESSOR_ARCHITEW6432=AMD64

C:\scratch\console>"c:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\MSBuild.exe" test.vcxproj /t:Rebuild /m /p:Configuration=Debug;Platform=Win32;UseNativeEnvironment=true -verbosity:diag | findstr PreferredToolArchitecture
                   PreferredToolArchitecture = x64

Whereas for a 32-bit command prompt on a 32-bit OS, UseNativeEnvironment = true won't force 64-bit tools to be used (when they are not available):

C:\scratch\console>set PROCESSOR_ARCH
PROCESSOR_ARCHITECTURE=x86
PROCESSOR_ARCHITEW6432=AMD64

C:\scratch\console>set PROCESSOR_ARCHITEW6432=

C:\scratch\console>set PROCESSOR_ARCH
PROCESSOR_ARCHITECTURE=x86

C:\scratch\console>"c:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\MSBuild.exe" test.vcxproj /t:Rebuild /m /p:Configuration=Debug;Platform=Win32;UseNativeEnvironment=true -verbosity:diag | findstr PreferredToolArchitecture
                   Property reassignment: $(PreferredToolArchitecture)="x86" (previous value: "x64") at c:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Microsoft\VC\v170\Microsoft.Cpp.ToolsetLocation.props (80,5)
                   PreferredToolArchitecture = x86

The net result of this is that since UseNativeEnvironment = '' by default, I think #1304 will have changed the behaviour of x86 builds so that they now use 32-bit tools instead of 64-bit ones.

Since, as per the description of #1302, the purpose of setting PreferredToolArchitecture = 'x64' was to handle large PCH files and prevent OOMs for x86 target builds, should you consider reversing #1304?

Thanks, Andrew R

microsoft / cppwinrt