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

MSVC ARM64 Compilation Error Due to Unsupported Inline Assembly #17665

Closed
dsmith111 opened this issue Jul 30, 2024 · 1 comment
Closed

MSVC ARM64 Compilation Error Due to Unsupported Inline Assembly #17665

dsmith111 opened this issue Jul 30, 2024 · 1 comment
Assignees
Labels

Comments

@dsmith111
Copy link
Contributor

dsmith111 commented Jul 30, 2024

What version of protobuf and what language are you using?
Version: main
Language: C++

What operating system (Linux, Windows, ...) and version?
Windows 11, ARM64

What runtime / compiler are you using (e.g., python version or gcc version)
MSVC
Microsoft (R) C/C++ Optimizing Compiler Version 19.38.33139

What did you do?
Steps to reproduce the behavior:

  1. Build with MSVC for a Windows ARM64 device.

What did you expect to see
Successful build.

What did you see instead?
google\protobuf\parse_context.h(721,10): error C2059: syntax error: ':'

google\protobuf\parse_context.h(898,14): error C3861: 'Ubfx7': identifier not found

Make sure you include information that can help us debug (full error message, exception listing, stack trace, logs).

https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/parse_context.h
Line 638 for potential patch.
Lines 721, 728 for breaking inline assembly.

I have noticed that other usages of inline assembly occur when either x86 architecture is defined, or when GNUC is defined. This line seems to be the only case that does not follow this. I'm not sure if this was intentional, or if #if defined(__aarch64__) && defined(__GNUC__) just happened to be missed during the addition. Could also use !defined(_MSC_VER), if it was intentional for clang and gnu.

Additionally, ARM-specific assembly instructions, such as "UBFX" also fails with MSVC.

@dsmith111 dsmith111 added the untriaged auto added to all issues by default when created. label Jul 30, 2024
@shaod2 shaod2 self-assigned this Aug 6, 2024
@shaod2 shaod2 added windows and removed untriaged auto added to all issues by default when created. labels Aug 6, 2024
copybara-service bot pushed a commit that referenced this issue Aug 15, 2024
…4 for Compatibility with MSVC (#17671)

#17665

**Problem**
`google\protobuf\parse_context.h(721,10): error C2059: syntax error: ':'`
`google\protobuf\parse_context.h(898,14): error C3861: 'Ubfx7': identifier not found`

The latest protobuf versions fail to compile with MSVC on Windows ARM64 architecture.

> Inline assembly is only available for x86 targets. For similar functionality in x64 or ARM64 code, use [compiler intrinsics](https://learn.microsoft.com/en-us/cpp/intrinsics/compiler-intrinsics?view=msvc-170).

https://learn.microsoft.com/en-us/cpp/assembler/inline/writing-functions-with-inline-assembly?view=msvc-170

**Solution**
As inline assembly for win ARM64 with MSVC as a compiler is not supported (even ARM specific assembly functions), a term has been added to the conditions in parse_context.h to prevent the assembly optimizations for arm64 from executing if the compiler is MSVC.

Tested by compiling protobuf with MSVC on Windows ARM64 and AMD64 architecture. Compilation was a success.

Closes #17671

COPYBARA_INTEGRATE_REVIEW=#17671 from dsmith111:smithdavi/msvc-compatibility-patch 765f8c8
PiperOrigin-RevId: 663262760
@dsmith111
Copy link
Contributor Author

Fix merged in here: c5f6231

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants