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

[clang] Allow __attribute__((swiftcall)) on all targets #71986

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kubamracek
Copy link
Member

With Embedded Swift, https://forums.swift.org/t/embedded-swift/67057, it becomes possible and useful to produce Swift code for a whole range of CPU targets. This change allows using the 'swiftcall' and 'swiftasynccall)' attributes on any Clang supported target. This isn't in any way adding complete support for those calling conventions for all targets, it's merely removing one obstacle from adding new targets to Swift.

With Embedded Swift, <https://forums.swift.org/t/embedded-swift/67057>, it
becomes possible and useful to produce Swift code for a whole range of CPU
targets. This change allows using the 'swiftcall' and 'swiftasynccall)'
attributes on any Clang supported target. This isn't in any way adding complete
support for those calling conventions for all targets, it's merely removing one
obstacle from adding new targets to Swift.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Nov 10, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 10, 2023

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Kuba (Brecka) Mracek (kubamracek)

Changes

With Embedded Swift, <https://forums.swift.org/t/embedded-swift/67057>, it becomes possible and useful to produce Swift code for a whole range of CPU targets. This change allows using the 'swiftcall' and 'swiftasynccall)' attributes on any Clang supported target. This isn't in any way adding complete support for those calling conventions for all targets, it's merely removing one obstacle from adding new targets to Swift.


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

4 Files Affected:

  • (modified) clang/include/clang/Basic/TargetInfo.h (+2)
  • (modified) clang/lib/CodeGen/ABIInfo.cpp (+4)
  • (modified) clang/lib/CodeGen/ABIInfo.h (+1)
  • (modified) clang/lib/CodeGen/TargetInfo.cpp (+3-1)
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 41f3c2e403cbef6..308d2ae830bf263 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1606,6 +1606,8 @@ class TargetInfo : public TransferrableTargetInfo,
       default:
         return CCCR_Warning;
       case CC_C:
+      case CC_Swift:
+      case CC_SwiftAsync:
         return CCCR_OK;
     }
   }
diff --git a/clang/lib/CodeGen/ABIInfo.cpp b/clang/lib/CodeGen/ABIInfo.cpp
index 1b56cf7c596d067..86897504b0ad267 100644
--- a/clang/lib/CodeGen/ABIInfo.cpp
+++ b/clang/lib/CodeGen/ABIInfo.cpp
@@ -33,6 +33,10 @@ const CodeGenOptions &ABIInfo::getCodeGenOpts() const {
   return CGT.getCodeGenOpts();
 }
 
+CodeGen::CodeGenTypes &ABIInfo::getCodeGenTypes() const {
+  return CGT;
+}
+
 bool ABIInfo::isAndroid() const { return getTarget().getTriple().isAndroid(); }
 
 bool ABIInfo::isOHOSFamily() const {
diff --git a/clang/lib/CodeGen/ABIInfo.h b/clang/lib/CodeGen/ABIInfo.h
index b9a5ef6e4366936..99b262bdb529684 100644
--- a/clang/lib/CodeGen/ABIInfo.h
+++ b/clang/lib/CodeGen/ABIInfo.h
@@ -60,6 +60,7 @@ class ABIInfo {
   const llvm::DataLayout &getDataLayout() const;
   const TargetInfo &getTarget() const;
   const CodeGenOptions &getCodeGenOpts() const;
+  CodeGen::CodeGenTypes &getCodeGenTypes() const;
 
   /// Return the calling convention to use for system runtime
   /// functions.
diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp
index 3d79f92137abc79..fdcf77ef9ce547c 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -67,7 +67,9 @@ LLVM_DUMP_METHOD void ABIArgInfo::dump() const {
 }
 
 TargetCodeGenInfo::TargetCodeGenInfo(std::unique_ptr<ABIInfo> Info)
-    : Info(std::move(Info)) {}
+    : Info(std::move(Info)),
+      SwiftInfo(std::make_unique<SwiftABIInfo>(
+          this->Info->getCodeGenTypes(), /*SwiftErrorInRegister*/ false)) {}
 
 TargetCodeGenInfo::~TargetCodeGenInfo() = default;
 

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 199fc973ced20016b04ba540cf63a1d4914fa513 d2df2830b4417c0ecc9690c4092053bf465a5d44 -- clang/include/clang/Basic/TargetInfo.h clang/lib/CodeGen/ABIInfo.cpp clang/lib/CodeGen/ABIInfo.h clang/lib/CodeGen/TargetInfo.cpp
View the diff from clang-format here.
diff --git a/clang/lib/CodeGen/ABIInfo.cpp b/clang/lib/CodeGen/ABIInfo.cpp
index 86897504b..4608535a1 100644
--- a/clang/lib/CodeGen/ABIInfo.cpp
+++ b/clang/lib/CodeGen/ABIInfo.cpp
@@ -33,9 +33,7 @@ const CodeGenOptions &ABIInfo::getCodeGenOpts() const {
   return CGT.getCodeGenOpts();
 }
 
-CodeGen::CodeGenTypes &ABIInfo::getCodeGenTypes() const {
-  return CGT;
-}
+CodeGen::CodeGenTypes &ABIInfo::getCodeGenTypes() const { return CGT; }
 
 bool ABIInfo::isAndroid() const { return getTarget().getTriple().isAndroid(); }
 

@s-barannikov
Copy link
Contributor

I think the choice whether or not to support Swift calling convention should be made by individual targets without providing "safe" default. As pointed out in the linked PR, supporting Swift CC is not only about enabling the attribute support -- backend also needs modifications so that the calling convention is effective. This can only be done based on the ABI, which is target-dependent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants