Port MSVC-internal changes #1344
Conversation
stl/msbuild/stl_2/md/msvcp_2_app/msvcp_2.nativeproj Fix indentation. stl/msbuild/stl_atomic_wait/stl_atomic_wait.files.settings.targets Strip trailing whitespace. stl/src/special_math.cpp Cleanup comments.
| #undef new | ||
|
|
||
| #if (defined(_M_IX86) || defined(_M_X64)) && !defined(_M_CEE_PURE) && !defined(_M_HYBRID) | ||
| #if (defined(_M_IX86) || defined(_M_X64) && !defined(_M_ARM64EC)) && !defined(_M_CEE_PURE) && !defined(_M_HYBRID) |
There was a problem hiding this comment.
I think either _M_HYBRID should moved to be next to _M_IX86 or _M_ARM64EC should be moved to the end of this line.
There was a problem hiding this comment.
I agree. I'm going to go ahead and merge this PR because getting MSVC and GitHub back in sync is critical, but I'll investigate this for a followup PR (while I am generally reluctant to mess with this stuff, this seems fairly straightforward and if it causes problems they will be immediately obvious, hopefully).
| // Do not include or define anything else here. | ||
| // In particular, basic_string must not be included here. | ||
| #if (defined(_M_IX86) || defined(_M_X64)) && !defined(_M_CEE_PURE) | ||
| #if (defined(_M_IX86) || defined(_M_X64) && !defined(_M_ARM64EC)) && !defined(_M_CEE_PURE) |
There was a problem hiding this comment.
I'm vaguely surprised that this line doesn't require a !defined(_M_HYBRID). Maybe I'm missing something in my mental model on how ARM64EC is supposed to work.
There was a problem hiding this comment.
I think the reason is that vector_algorithms.cpp was added to the list of files to compile for EC, but I bet it's not being compiled at all for Hybrid. (My understanding of these is near-zero.) I can investigate this as a cleanup for a later PR.
Followup to microsoftGH-1344.
|
Not complaining or anything, but adding a short description of what the patch does in the comit message for people that don't have access to the internal repository would be nice. |
|
Yeah, I really phoned it in on this one, sorry about that. I tried to summarize the story in this PR's description (we've generally been writing detailed PR descriptions, but not copying them to commit messages, under the theory that the PR number is enough to find the details) but even that summary is very vague - because I understood few of the details myself. The background is that the MSVC-internal repo has two major branches, After porting this today, I asked the BE devs for more info, so now I understand it in a little more detail than "mysterious architecture macros" which was my earlier understanding. This is related to the x64 emulation on ARM64 that we announced last week; EC means "emulation compatible". There are a lot of changes to the MS-internal build system here (which don't yet affect the GitHub CMake/Ninja build system because our migration work is ongoing - we would like to stop using MSBuild as soon as possible but C++20 is keeping us busy) and a few changes to product code, especially where intrinsics are involved (e.g. even when emulating code with compiler magic, atomics really need to care about ARM64's memory model). |
No worries, I was just curious. You can probably not give an answer to this, but that sounds as if one would have to compile in a special mode, if a program is supposed to run on a Windows on ARM device via x64 Emulation, which would kind of defeat the purpose of the emulation. Or is this behind a runtime dispatcher? |
|
I am far from qualified to answer that, but my understanding is that "normal" x64 programs will automatically work; this ARM64EC stuff is intended for OS binaries and libraries like the STL that need to do special things like atomics. (Observe that the vast majority of the STL's code doesn't care about this macro, just 32/64-bit.) |
Followup to microsoftGH-1344. Code review feedback.
4 participants
This ports Microsoft-internal MSVC-PR-279169, which merged accumulated work from the compiler back-end team in their
prod/beMSVC branch. After applying thegit diff --binarypatch with no conflicts or manual edits, I verified that the GitHub and MSVC repos are binary-identical. (Note that I did not copy-overwrite, which would pick up unintentional divergence; this was a proper patch, as always.)