Ensure that the correct VC++ toolset is selected in all build environments

[eiriktsarpalis]

Follow-up on #104695, this PR tries to set the correct build architectures for all configurations. The changes result in cross-compilation failing in the gen-buildsys.cmd stage, figured I'd post the PR for group troubleshooting.

Fix #104674. Fix #76516.

[dotnet-policy-service]

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

[MichalStrehovsky]

Looks great, thanks. I think this is also fixing #76516!

[am11]

Same change in src/coreclr/nativeaot/BuildIntegration/findvcvarsall.bat?

[eiriktsarpalis]

Same change in src/coreclr/nativeaot/BuildIntegration/findvcvarsall.bat?

Where is it being used?

[am11]

Where is it being used?

dotnet publish -p:PublishAot=true --runtime win-arm64 etc.

[eiriktsarpalis]

Are you suggesting that the script is being used by the product? (rather than building the product?)

[MichalStrehovsky]

Same change in src/coreclr/nativeaot/BuildIntegration/findvcvarsall.bat?

Where is it being used?

This is tracked in #76079.

If you'd like to volunteer your time for that one too, we'd need to make the change and then run this on a arm64 machine to validate (I assume both build.cmd will build arm64 flavors):

build.cmd clr.aot+libs -c Release
src\tests\build.cmd nativeaot release tree nativeaot
src\tests\run.cmd runnativeaottests release

It should be an easy fix, we've just not done it because Microsoft doesn't want to spend money on giving me an ARM64 machine and I will not spend my mental health on using an ARM64 machine on a different continent interactively.

It doesn't have to be part of this PR, it's not an infra change, it's a product change.

[am11]

I have win-arm64 VM running on osx-arm64, I can take a look based on @eiriktsarpalis changes (unless Eirik beats me to it 😄).

[eiriktsarpalis]

I expect some of the assumptions we're making for the build (e.g. #104763 (comment)) might be different in the context of the product, so we should keep the changes in separate PRs. @am11 feel free to have a go at this.

[eiriktsarpalis]

@am11 what certainly seems to be the case is that we should be adding provision for 32-bit machines that this PR is skipping.

[am11]

BuildIntegration script is used by both, product build as well as shipped in ILCompiler nuget package for windows. Quick testing suggests this should do the trick: am11@33ce9b3.

[jkotas]

BuildIntegration script is used by both, product build as well as shipped in ILCompiler nuget package for windows. Quick testing suggests this should do the trick: am11@33ce9b3.

I do not think we need to bother with 32-bit Windows x86 machines. Those machines do not really exist anymore as dev machines and Visual Studio (that is a pre-req for naor publish) requires 64-bit. I would keep it simple and always use 64-bit tools, same as what we do in the repo build.

[am11]

I would keep it simple and always use 64-bit tools, same as what we do in the repo build.

main...am11:patch-5

I think -requires Microsoft.VisualStudio.Component.VC.Tools.%toolsSuffix% is something we should use with vswhere in other places as well (instead of hardcoded x86.x64).

[hez2010]

Can you also include a fix to

runtime/eng/native/configurecompiler.cmake

Line 999 in f9eda07

file(TO_CMAKE_PATH "$ENV{VCToolsInstallDir}\\bin\\HostX86\\arm64\\armasm64.exe" CMAKE_ASM_COMPILER)

before closing #104674?
It needs to use $ENV{VCToolsInstallDir}\\bin\\Hostarm64\\arm64\\armasm64.exe instead of $ENV{VCToolsInstallDir}\\bin\\HostX86\\arm64\\armasm64.exe.
And CLR_CMAKE_HOST_WIN32+CLR_CMAKE_HOST_ARCH_ARM isn't a thing now so the branch can be removed:

runtime/eng/native/configurecompiler.cmake

Lines 979 to 995 in f9eda07

	if(CLR_CMAKE_HOST_ARCH_ARM)
	# Explicitly specify the assembler to be used for Arm32 compile
	file(TO_CMAKE_PATH "$ENV{VCToolsInstallDir}\\bin\\HostX86\\arm\\armasm.exe" CMAKE_ASM_COMPILER)
	set(CMAKE_ASM_MASM_COMPILER ${CMAKE_ASM_COMPILER})
	message("CMAKE_ASM_MASM_COMPILER explicitly set to: ${CMAKE_ASM_MASM_COMPILER}")
	# Enable generic assembly compilation to avoid CMake generate VS proj files that explicitly
	# use ml[64].exe as the assembler.
	enable_language(ASM)
	set(CMAKE_ASM_COMPILE_OPTIONS_MSVC_RUNTIME_LIBRARY_MultiThreaded "")
	set(CMAKE_ASM_COMPILE_OPTIONS_MSVC_RUNTIME_LIBRARY_MultiThreadedDLL "")
	set(CMAKE_ASM_COMPILE_OPTIONS_MSVC_RUNTIME_LIBRARY_MultiThreadedDebug "")
	set(CMAKE_ASM_COMPILE_OPTIONS_MSVC_RUNTIME_LIBRARY_MultiThreadedDebugDLL "")
	set(CMAKE_ASM_COMPILE_OBJECT "<CMAKE_ASM_COMPILER> -g <INCLUDES> <FLAGS> -o <OBJECT> <SOURCE>")

[eiriktsarpalis]

@hez2010 is it a question of simply updating the path or do we need to condition this on the build architecture?

[hez2010]

@hez2010 is it a question of simply updating the path or do we need to condition this on the build architecture?

Simply updating the path should be okay. It has already been guarded by both CLR_CMAKE_HOST_WIN32 and CLR_CMAKE_HOST_ARCH_ARM64.

[eiriktsarpalis]

That would still be entered if you're cross compiling to arm64 on x64 though, right?

[eiriktsarpalis]

I updated the assembler logic as well, but it turns out that some branching is required for cross-compilation on x64 to keep working.