Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support x64-only symbols on ARM64EC #3732

Merged
merged 2 commits into from May 30, 2023

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented May 25, 2023

  • Adds support for building the STL for the ARM64EC platform. (No testing yet, just build.)
  • Enable the __std_init_once_meow ALTERNATENAMEs in src/xonce2.cpp on ARM64EC. Note that we continue not to use the <mutex> ALTERNATENAMEs when compiling for ARM64EC - the linkage model is complicated - we only include the ALTERNATENAMEs to support linking with object files built targeting x64. For example, linking x64 static libraries into an ARM64EC binary needs to work.
  • Isolate x86/x64-specific code in src/vector_algorithms.cpp so we can enable the non-vectorized fallback implementations for ARM64EC. Again, these aren't used when compiling for ARM64EC, but we need the symbols in the import lib to support linking with x64 object files.

Fixes #3382

Note that there's no test coverage here. We're close-but-not-ready to support testing of the STL on ARM64, but a ways away from being able to compile an object file for one platform and link it into a binary built for another. I did validate the fix on an ARM64 VM using files test.cpp:

#include <cassert>
#include <cstddef>

extern int f(const char* ptr, std::size_t length);
extern int g();

int main() {
    static const char data[] = "this is a test, this is only a test";
    assert(f(data, sizeof(data)) == 14);
    assert(g() == 42);
}

and repro.cpp:

#include <algorithm>
#include <cstddef>
#include <mutex>

int f(const char* ptr, std::size_t length) {
    return std::find(ptr, ptr + length, ',') - ptr;
}

int g() {
    static std::once_flag nonce;
    static int x = 0;
    std::call_once(nonce, [] { x = 42; });
    return x;
}

compiling repro.cpp for x64, and linking the resulting object with test.cpp (EDIT: built with /arm64EC, of course). I performed this process once for each of /MT, /MTd, /MD, and /MDd and verified the resulting program linked and ran successfully. (Debug should make no difference, but paranoia never hurts.)

* Adds support for building the STL for the ARM64EC platform. (No testing yet, just build.)
* Enable the `__std_init_once_meow` `ALTERNATENAME`s in `src/xonce2.cpp` on ARM64EC. Note that we continue not to use the `<mutex>` `ALTERNATENAME`s when compiling for ARM64EC - the linkage model is complicated - we only include the `ALTERNATENAME`s to support linking with object files built targeting x64. For example, linking x64 static libraries into an ARM64EC binary needs to work.
* Isolate x86/x64-specific code in `src/vector_algorithms.cpp` so we can enable the non-vectorized fallback implementations for ARM64EC. Again, these aren't used when compiling for ARM64EC, but we need the symbols in the import lib to support linking with x64 object files.

Fixes microsoft#3382
@CaseyCarter CaseyCarter added the bug Something isn't working label May 25, 2023
@CaseyCarter CaseyCarter requested a review from a team as a code owner May 25, 2023 20:21
@github-actions github-actions bot added this to Initial Review in Code Reviews May 25, 2023
@CaseyCarter CaseyCarter added high priority Important! ARM64 Related to the ARM64 architecture labels May 25, 2023
CMakeLists.txt Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in Code Reviews May 26, 2023
@StephanTLavavej StephanTLavavej moved this from Final Review to Ready To Merge in Code Reviews May 26, 2023
@StephanTLavavej StephanTLavavej self-assigned this May 26, 2023
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit d08d14c into microsoft:main May 30, 2023
35 checks passed
Code Reviews automation moved this from Ready To Merge to Done May 30, 2023
@StephanTLavavej
Copy link
Member

Thanks for figuring out how to fix this major bug that was affecting several customers! 🛠️ 💡 😻

@CaseyCarter CaseyCarter deleted the arm64ec branch May 31, 2023 19:28
@kobykahane
Copy link

Will this be backported to 17.6/17.7?

@StephanTLavavej
Copy link
Member

At this time, we have no plans to backport the fix. There's an escape hatch of disabling the vectorized algorithms when compiling the x64 object files.

@kobykahane
Copy link

At this time, we have no plans to backport the fix. There's an escape hatch of disabling the vectorized algorithms when compiling the x64 object files.

Thanks @StephanTLavavej. Unfortunately, in our case, we're not in a position to recompile the x64 static libraries we're attempting to consume in our ARM64EC build. If we were, we'd probably just rebuild them as ARM64 and wouldn't have opted for EC. Additionally, we're hitting the __std_init_once_* cases, to which disabling the vectorized algorithms doesn't seem to apply (although it seems that manually defining the alternate names could take care of that one).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARM64 Related to the ARM64 architecture bug Something isn't working high priority Important!
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Errors when linking x64 OBJ files into ARM64EC programs
3 participants