[ARM64EC] Add support for parsing __vectorcall #87725

efriedma-quic · 2024-04-04T23:19:21Z

MSVC doesn't support generating __vectorcall calls in Arm64EC mode, but it does treat it as a distinct type. The Microsoft STL depends on this functionality. (Not sure if this is intentional.) Add support for parsing the same way as MSVC, and add some checks to ensure we don't try to actually generate code.

The error handling in CodeGen is ugly, but I can't think of a better way to do it.

MSVC doesn't support generating __vectorcall calls in Arm64EC mode, but it does treat it as a distinct type. The Microsoft STL depends on this functionality. (Not sure if this is intentional.) Add support for parsing the same way as MSVC, and add some checks to ensure we don't try to actually generate code. The error handling in CodeGen is ugly, but I can't think of a better way to do it.

llvmbot · 2024-04-04T23:19:51Z

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-backend-aarch64

Author: Eli Friedman (efriedma-quic)

Changes

MSVC doesn't support generating __vectorcall calls in Arm64EC mode, but it does treat it as a distinct type. The Microsoft STL depends on this functionality. (Not sure if this is intentional.) Add support for parsing the same way as MSVC, and add some checks to ensure we don't try to actually generate code.

The error handling in CodeGen is ugly, but I can't think of a better way to do it.

Full diff: https://github.com/llvm/llvm-project/pull/87725.diff

4 Files Affected:

(modified) clang/lib/Basic/Targets/AArch64.cpp (+4-1)
(modified) clang/lib/CodeGen/CGCall.cpp (+6)
(modified) clang/lib/CodeGen/CodeGenModule.cpp (+8)
(added) clang/test/CodeGenCXX/arm64ec-vectorcall.cpp (+14)

diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 1569b5e04b770a..c8d243a8fb7aea 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -1543,10 +1543,13 @@ WindowsARM64TargetInfo::getBuiltinVaListKind() const {
 TargetInfo::CallingConvCheckResult
 WindowsARM64TargetInfo::checkCallingConvention(CallingConv CC) const {
   switch (CC) {
+  case CC_X86VectorCall:
+    if (getTriple().isWindowsArm64EC())
+      return CCCR_OK;
+    return CCCR_Ignore;
   case CC_X86StdCall:
   case CC_X86ThisCall:
   case CC_X86FastCall:
-  case CC_X86VectorCall:
     return CCCR_Ignore;
   case CC_C:
   case CC_OpenCLKernel:
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index f12765b826935b..3f5463a9a70e9d 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5591,6 +5591,12 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
                              /*AttrOnCallSite=*/true,
                              /*IsThunk=*/false);
 
+  if (CallingConv == llvm::CallingConv::X86_VectorCall &&
+      getTarget().getTriple().isWindowsArm64EC()) {
+    CGM.Error(Loc, "__vectorcall calling convention is not currently "
+                   "supported");
+  }
+
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CurFuncDecl)) {
     if (FD->hasAttr<StrictFPAttr>())
       // All calls within a strictfp function are marked strictfp
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 00b3bfcaa0bc25..75519be8bba052 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2087,6 +2087,14 @@ void CodeGenModule::SetLLVMFunctionAttributes(GlobalDecl GD,
   llvm::AttributeList PAL;
   ConstructAttributeList(F->getName(), Info, GD, PAL, CallingConv,
                          /*AttrOnCallSite=*/false, IsThunk);
+  if (CallingConv == llvm::CallingConv::X86_VectorCall &&
+      getTarget().getTriple().isWindowsArm64EC()) {
+    SourceLocation Loc;
+    if (const Decl *D = GD.getDecl())
+      Loc = D->getLocation();
+
+    Error(Loc, "__vectorcall calling convention is not currently supported");
+  }
   F->setAttributes(PAL);
   F->setCallingConv(static_cast<llvm::CallingConv::ID>(CallingConv));
 }
diff --git a/clang/test/CodeGenCXX/arm64ec-vectorcall.cpp b/clang/test/CodeGenCXX/arm64ec-vectorcall.cpp
new file mode 100644
index 00000000000000..73d2d63835917c
--- /dev/null
+++ b/clang/test/CodeGenCXX/arm64ec-vectorcall.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple arm64ec-windows-msvc -emit-llvm -o - %s -verify
+
+// ARM64EC doesn't support generating __vectorcall calls... but __vectorcall
+// function types need to be distinct from __cdecl function types to support
+// compiling the STL. Make sure we only diagnose constructs that actually
+// require generating code.
+void __vectorcall f1();
+void f2(void __vectorcall p()) {}
+void f2(void p()) {}
+void __vectorcall (*f3)();
+void __vectorcall f4(); // expected-error {{__vectorcall}}
+void __vectorcall f5() { // expected-error {{__vectorcall}}
+  f4(); // expected-error{{__vectorcall}}
+}

llvmbot · 2024-04-04T23:19:51Z

@llvm/pr-subscribers-clang

Author: Eli Friedman (efriedma-quic)

Changes

MSVC doesn't support generating __vectorcall calls in Arm64EC mode, but it does treat it as a distinct type. The Microsoft STL depends on this functionality. (Not sure if this is intentional.) Add support for parsing the same way as MSVC, and add some checks to ensure we don't try to actually generate code.

The error handling in CodeGen is ugly, but I can't think of a better way to do it.

Full diff: https://github.com/llvm/llvm-project/pull/87725.diff

4 Files Affected:

(modified) clang/lib/Basic/Targets/AArch64.cpp (+4-1)
(modified) clang/lib/CodeGen/CGCall.cpp (+6)
(modified) clang/lib/CodeGen/CodeGenModule.cpp (+8)
(added) clang/test/CodeGenCXX/arm64ec-vectorcall.cpp (+14)

diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 1569b5e04b770a..c8d243a8fb7aea 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -1543,10 +1543,13 @@ WindowsARM64TargetInfo::getBuiltinVaListKind() const {
 TargetInfo::CallingConvCheckResult
 WindowsARM64TargetInfo::checkCallingConvention(CallingConv CC) const {
   switch (CC) {
+  case CC_X86VectorCall:
+    if (getTriple().isWindowsArm64EC())
+      return CCCR_OK;
+    return CCCR_Ignore;
   case CC_X86StdCall:
   case CC_X86ThisCall:
   case CC_X86FastCall:
-  case CC_X86VectorCall:
     return CCCR_Ignore;
   case CC_C:
   case CC_OpenCLKernel:
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index f12765b826935b..3f5463a9a70e9d 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5591,6 +5591,12 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
                              /*AttrOnCallSite=*/true,
                              /*IsThunk=*/false);
 
+  if (CallingConv == llvm::CallingConv::X86_VectorCall &&
+      getTarget().getTriple().isWindowsArm64EC()) {
+    CGM.Error(Loc, "__vectorcall calling convention is not currently "
+                   "supported");
+  }
+
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CurFuncDecl)) {
     if (FD->hasAttr<StrictFPAttr>())
       // All calls within a strictfp function are marked strictfp
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 00b3bfcaa0bc25..75519be8bba052 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2087,6 +2087,14 @@ void CodeGenModule::SetLLVMFunctionAttributes(GlobalDecl GD,
   llvm::AttributeList PAL;
   ConstructAttributeList(F->getName(), Info, GD, PAL, CallingConv,
                          /*AttrOnCallSite=*/false, IsThunk);
+  if (CallingConv == llvm::CallingConv::X86_VectorCall &&
+      getTarget().getTriple().isWindowsArm64EC()) {
+    SourceLocation Loc;
+    if (const Decl *D = GD.getDecl())
+      Loc = D->getLocation();
+
+    Error(Loc, "__vectorcall calling convention is not currently supported");
+  }
   F->setAttributes(PAL);
   F->setCallingConv(static_cast<llvm::CallingConv::ID>(CallingConv));
 }
diff --git a/clang/test/CodeGenCXX/arm64ec-vectorcall.cpp b/clang/test/CodeGenCXX/arm64ec-vectorcall.cpp
new file mode 100644
index 00000000000000..73d2d63835917c
--- /dev/null
+++ b/clang/test/CodeGenCXX/arm64ec-vectorcall.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple arm64ec-windows-msvc -emit-llvm -o - %s -verify
+
+// ARM64EC doesn't support generating __vectorcall calls... but __vectorcall
+// function types need to be distinct from __cdecl function types to support
+// compiling the STL. Make sure we only diagnose constructs that actually
+// require generating code.
+void __vectorcall f1();
+void f2(void __vectorcall p()) {}
+void f2(void p()) {}
+void __vectorcall (*f3)();
+void __vectorcall f4(); // expected-error {{__vectorcall}}
+void __vectorcall f5() { // expected-error {{__vectorcall}}
+  f4(); // expected-error{{__vectorcall}}
+}

efriedma-quic · 2024-04-04T23:20:26Z

CC @bylaws @mstorsjo @cjacek @MaxEW707 @CaseyCarter

MaxEW707 · 2024-04-07T20:41:27Z

LGTM.

From memory, its been a while since I've had to ship on ARM64EC, all instances of __vectorcall are supposed to be rejected by msvc. That also aligns with the __vectorcall docs on MSDN. I'll get a bug report up to the ms dev forums next week if you aren't planning to :).
I am not sure what clang's policy is with potentially allowing code that "should" be rejected by msvc but since this errors when the function is actually generated this should be fine to support even if msvc gets more strict in the future.

@CaseyCarter It feels like we are missing an arm64ec check here, https://github.com/microsoft/STL/blob/be81252ed1f5e5fc6d77faca0b5dbbbdae8143a2/stl/inc/type_traits#L398 unless this was intentional and just not documented?

CaseyCarter · 2024-04-16T17:15:38Z

@CaseyCarter It feels like we are missing an arm64ec check here, https://github.com/microsoft/STL/blob/be81252ed1f5e5fc6d77faca0b5dbbbdae8143a2/stl/inc/type_traits#L398 unless this was intentional and just not documented?

I have absolutely no idea what's going on here or why MSVC doesn't reject this usage. I've filed microsoft/STL#4596 to track investigating and (most likely) removal. Thanks!

llvmbot added clang Clang issues not falling into any other category backend:AArch64 clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Apr 4, 2024

MaxEW707 approved these changes Apr 7, 2024

View reviewed changes

cjacek approved these changes Apr 8, 2024

View reviewed changes

efriedma-quic merged commit 71097e9 into llvm:main Apr 10, 2024
9 checks passed

efriedma-quic deleted the arm64ec-vectorcall branch April 10, 2024 02:54

CaseyCarter mentioned this pull request Apr 16, 2024

<type_traits>: __vectorcall _should_ be rejected on ARM64EC but isn't microsoft/STL#4596

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ARM64EC] Add support for parsing __vectorcall #87725

[ARM64EC] Add support for parsing __vectorcall #87725

efriedma-quic commented Apr 4, 2024

llvmbot commented Apr 4, 2024 •

edited

llvmbot commented Apr 4, 2024

efriedma-quic commented Apr 4, 2024

MaxEW707 commented Apr 7, 2024 •

edited

CaseyCarter commented Apr 16, 2024 •

edited

[ARM64EC] Add support for parsing __vectorcall #87725

[ARM64EC] Add support for parsing __vectorcall #87725

Conversation

efriedma-quic commented Apr 4, 2024

llvmbot commented Apr 4, 2024 • edited

llvmbot commented Apr 4, 2024

efriedma-quic commented Apr 4, 2024

MaxEW707 commented Apr 7, 2024 • edited

CaseyCarter commented Apr 16, 2024 • edited

llvmbot commented Apr 4, 2024 •

edited

MaxEW707 commented Apr 7, 2024 •

edited

CaseyCarter commented Apr 16, 2024 •

edited