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

[CodeGen][ARM64EC] Use alias symbol for exporting hybrid_patchable functions. #100872

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

cjacek
Copy link
Contributor

@cjacek cjacek commented Jul 27, 2024

Exporting $hp_target symbol doesn't make sense, use the unmangled alias instead. This is not compatible with MSVC, but it makes using dllexport together with hybrid_patchable attribute possible.

This provides a possible fix for #93910.

@llvmbot
Copy link
Member

llvmbot commented Jul 27, 2024

@llvm/pr-subscribers-backend-aarch64

Author: Jacek Caban (cjacek)

Changes

Exporting $hp_target symbol doesn't make sense, use the unmangled alias instead. This is not compatible with MSVC, but it makes using dllexport together with hybrid_patchable attribute possible.

This provides a possible fix for #93910.


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

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp (+5)
  • (modified) llvm/test/CodeGen/AArch64/arm64ec-hybrid-patchable.ll (+1-1)
diff --git a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
index 310b152ef9817..415edb189e60c 100644
--- a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
@@ -833,6 +833,11 @@ bool AArch64Arm64ECCallLowering::runOnModule(Module &Mod) {
                                               "EXP+" + MangledName.value())));
       A->setAliasee(&F);
 
+      if (F.hasDLLExportStorageClass()) {
+        A->setDLLStorageClass(GlobalValue::DLLExportStorageClass);
+        F.setDLLStorageClass(GlobalValue::DefaultStorageClass);
+      }
+
       FnsMap[A] = GlobalAlias::create(GlobalValue::LinkOnceODRLinkage,
                                       MangledName.value(), &F);
       PatchableFns.insert(A);
diff --git a/llvm/test/CodeGen/AArch64/arm64ec-hybrid-patchable.ll b/llvm/test/CodeGen/AArch64/arm64ec-hybrid-patchable.ll
index e5387d40b9c64..64fb5b36b2c62 100644
--- a/llvm/test/CodeGen/AArch64/arm64ec-hybrid-patchable.ll
+++ b/llvm/test/CodeGen/AArch64/arm64ec-hybrid-patchable.ll
@@ -238,7 +238,7 @@ define dso_local void @caller() nounwind {
 ; CHECK-NEXT:      .symidx exp
 ; CHECK-NEXT:      .word   0
 ; CHECK-NEXT:      .section        .drectve,"yni"
-; CHECK-NEXT:      .ascii  " /EXPORT:\"#exp$hp_target,EXPORTAS,exp$hp_target\""
+; CHECK-NEXT:      .ascii  " /EXPORT:exp"
 
 ; CHECK-NEXT:      .def    func;
 ; CHECK-NEXT:      .scl    2;

Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

…nctions.

Exporting $hp_target symbol doesn't make sense, use the unmangled alias instead.
This is not compatible with MSVC, but it makes using dllexport together with
hybrid_patchable attribute possible.
@tru tru force-pushed the patchable-dllexport branch from 84188e7 to ed4ef4d Compare July 30, 2024 06:30
@cjacek cjacek merged commit 41c0f89 into llvm:main Jul 30, 2024
7 checks passed
@cjacek cjacek deleted the patchable-dllexport branch July 30, 2024 12:22
@cjacek
Copy link
Contributor Author

cjacek commented Jul 30, 2024

/cherry-pick 41c0f89

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 30, 2024
…nctions. (llvm#100872)

Exporting $hp_target symbol doesn't make sense, use the unmangled alias instead.
This is not compatible with MSVC, but it makes using dllexport together with
hybrid_patchable attribute possible.

(cherry picked from commit 41c0f89)
@llvmbot
Copy link
Member

llvmbot commented Jul 30, 2024

/pull-request #101178

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 31, 2024
…nctions. (llvm#100872)

Exporting $hp_target symbol doesn't make sense, use the unmangled alias instead.
This is not compatible with MSVC, but it makes using dllexport together with
hybrid_patchable attribute possible.

(cherry picked from commit 41c0f89)
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
…nctions. (llvm#100872)

Exporting $hp_target symbol doesn't make sense, use the unmangled alias instead.
This is not compatible with MSVC, but it makes using dllexport together with
hybrid_patchable attribute possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants