[IR] Use EXPORTAS for ARM64EC mangled symbols with dllexport attribute. #81940

cjacek · 2024-02-15T23:47:44Z

We currently just use mangled name. This works fine, because linker should detect that and demangle it for the export table. However, on MSVC, the compiler is more specific and passes demangled name as well, with EXPORTAS. This PR aims to match that. MSVC doesn't use quotes in this case, so I added '#' to the list of characters that don't need it.

llvmbot · 2024-02-15T23:48:15Z

@llvm/pr-subscribers-llvm-ir

Author: Jacek Caban (cjacek)

Changes

We currently just use mangled name. This works fine, because linker should detect that and demangle it for the export table. However, on MSVC, the compiler is more specific and passes demangled name as well, with EXPORTAS. This PR aims to match that. MSVC doesn't use quotes in this case, so I added '#' to the list of characters that don't need it.

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

2 Files Affected:

(modified) llvm/lib/IR/Mangler.cpp (+8-1)
(modified) llvm/test/CodeGen/AArch64/dllexport.ll (+39)

diff --git a/llvm/lib/IR/Mangler.cpp b/llvm/lib/IR/Mangler.cpp
index 3acac2c3e3db16..9d6a0ef6ec3aae 100644
--- a/llvm/lib/IR/Mangler.cpp
+++ b/llvm/lib/IR/Mangler.cpp
@@ -20,6 +20,8 @@
 #include "llvm/IR/Module.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Triple.h"
+#include "llvm/Object/COFF.h"
+
 using namespace llvm;
 
 namespace {
@@ -192,7 +194,7 @@ void Mangler::getNameWithPrefix(SmallVectorImpl<char> &OutName,
 
 // Check if the name needs quotes to be safe for the linker to interpret.
 static bool canBeUnquotedInDirective(char C) {
-  return isAlnum(C) || C == '_' || C == '@';
+  return isAlnum(C) || C == '_' || C == '@' || C == '#';
 }
 
 static bool canBeUnquotedInDirective(StringRef Name) {
@@ -233,6 +235,11 @@ void llvm::emitLinkerFlagsForGlobalCOFF(raw_ostream &OS, const GlobalValue *GV,
     } else {
       Mangler.getNameWithPrefix(OS, GV, false);
     }
+    if (TT.isWindowsArm64EC()) {
+      if (std::optional<std::string> demangledName =
+              object::getArm64ECDemangledFunctionName(GV->getName()))
+        OS << ",EXPORTAS," << *demangledName;
+    }
     if (NeedQuotes)
       OS << "\"";
 
diff --git a/llvm/test/CodeGen/AArch64/dllexport.ll b/llvm/test/CodeGen/AArch64/dllexport.ll
index 81ba674a0dedc1..5f2628619dcc3d 100644
--- a/llvm/test/CodeGen/AArch64/dllexport.ll
+++ b/llvm/test/CodeGen/AArch64/dllexport.ll
@@ -1,5 +1,7 @@
 ; RUN: llc -mtriple aarch64-windows-gnu -filetype asm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-GNU
 ; RUN: llc -mtriple aarch64-windows-msvc -filetype asm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-MSVC
+; RUN: llc -mtriple arm64ec-windows-gnu -filetype asm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-GNU-EC
+; RUN: llc -mtriple arm64ec-windows-msvc -filetype asm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-MSVC-EC
 
 define void @f() {
   ret void
@@ -71,3 +73,40 @@ define weak_odr dllexport void @l() {
 ; CHECK-MSVC: .ascii "  /EXPORT:s"
 ; CHECK-MSVC: .ascii "  /EXPORT:t"
 ; CHECK-MSVC: .ascii "  /EXPORT:u"
+
+; CHECK-GNU-NOT-EC: -export:f
+; CHECK-GNU-NOT-EC: -export:#f,EXPORTAS,f
+; CHECK-GNU-EC: .ascii " -export:#g,EXPORTAS,g
+; CHECK-GNU-EC: .ascii " -export:#h,EXPORTAS,h
+; CHECK-GNU-NOT-EC: -export:i
+; CHECK-GNU-NOT-EC: -export:#i,EXPORTAS,i
+; CHECK-GNU-EC: .ascii " -export:#j,EXPORTAS,j"
+; CHECK-GNU-EC: .ascii " -export:#k,EXPORTAS,k"
+; CHECK-GNU-EC: .ascii " -export:#l,EXPORTAS,l"
+; CHECK-GNU-EC: .ascii " -export:m,data"
+; CHECK-GNU-EC: .ascii " -export:n,data"
+; CHECK-GNU-EC: .ascii " -export:o,data"
+; CHECK-GNU-EC: .ascii " -export:p,data"
+; CHECK-GNU-EC: .ascii " -export:q,data"
+; CHECK-GNU-EC: .ascii " -export:r"
+; CHECK-GNU-EC: .ascii " -export:s"
+; CHECK-GNU-EC: .ascii " -export:t"
+; CHECK-GNU-EC: .ascii " -export:u"
+; CHECK-MSVC-NOT-EC: /EXPORT:f
+; CHECK-MSVC-NOT-EC: /EXPORT:#f,EXPORTAS,f
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#g,EXPORTAS,g"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#h,EXPORTAS,h"
+; CHECK-MSVC-NOT-EC: /EXPORT:i
+; CHECK-MSVC-NOT-EC: /EXPORT:#i,EXPORTAS,i
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#j,EXPORTAS,j"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#k,EXPORTAS,k"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#l,EXPORTAS,l"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:m,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:n,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:o,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:p,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:q,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:r"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:s"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:t"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:u"

llvmbot · 2024-02-15T23:48:15Z

@llvm/pr-subscribers-backend-aarch64

Author: Jacek Caban (cjacek)

Changes

We currently just use mangled name. This works fine, because linker should detect that and demangle it for the export table. However, on MSVC, the compiler is more specific and passes demangled name as well, with EXPORTAS. This PR aims to match that. MSVC doesn't use quotes in this case, so I added '#' to the list of characters that don't need it.

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

2 Files Affected:

(modified) llvm/lib/IR/Mangler.cpp (+8-1)
(modified) llvm/test/CodeGen/AArch64/dllexport.ll (+39)

diff --git a/llvm/lib/IR/Mangler.cpp b/llvm/lib/IR/Mangler.cpp
index 3acac2c3e3db16..9d6a0ef6ec3aae 100644
--- a/llvm/lib/IR/Mangler.cpp
+++ b/llvm/lib/IR/Mangler.cpp
@@ -20,6 +20,8 @@
 #include "llvm/IR/Module.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Triple.h"
+#include "llvm/Object/COFF.h"
+
 using namespace llvm;
 
 namespace {
@@ -192,7 +194,7 @@ void Mangler::getNameWithPrefix(SmallVectorImpl<char> &OutName,
 
 // Check if the name needs quotes to be safe for the linker to interpret.
 static bool canBeUnquotedInDirective(char C) {
-  return isAlnum(C) || C == '_' || C == '@';
+  return isAlnum(C) || C == '_' || C == '@' || C == '#';
 }
 
 static bool canBeUnquotedInDirective(StringRef Name) {
@@ -233,6 +235,11 @@ void llvm::emitLinkerFlagsForGlobalCOFF(raw_ostream &OS, const GlobalValue *GV,
     } else {
       Mangler.getNameWithPrefix(OS, GV, false);
     }
+    if (TT.isWindowsArm64EC()) {
+      if (std::optional<std::string> demangledName =
+              object::getArm64ECDemangledFunctionName(GV->getName()))
+        OS << ",EXPORTAS," << *demangledName;
+    }
     if (NeedQuotes)
       OS << "\"";
 
diff --git a/llvm/test/CodeGen/AArch64/dllexport.ll b/llvm/test/CodeGen/AArch64/dllexport.ll
index 81ba674a0dedc1..5f2628619dcc3d 100644
--- a/llvm/test/CodeGen/AArch64/dllexport.ll
+++ b/llvm/test/CodeGen/AArch64/dllexport.ll
@@ -1,5 +1,7 @@
 ; RUN: llc -mtriple aarch64-windows-gnu -filetype asm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-GNU
 ; RUN: llc -mtriple aarch64-windows-msvc -filetype asm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-MSVC
+; RUN: llc -mtriple arm64ec-windows-gnu -filetype asm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-GNU-EC
+; RUN: llc -mtriple arm64ec-windows-msvc -filetype asm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-MSVC-EC
 
 define void @f() {
   ret void
@@ -71,3 +73,40 @@ define weak_odr dllexport void @l() {
 ; CHECK-MSVC: .ascii "  /EXPORT:s"
 ; CHECK-MSVC: .ascii "  /EXPORT:t"
 ; CHECK-MSVC: .ascii "  /EXPORT:u"
+
+; CHECK-GNU-NOT-EC: -export:f
+; CHECK-GNU-NOT-EC: -export:#f,EXPORTAS,f
+; CHECK-GNU-EC: .ascii " -export:#g,EXPORTAS,g
+; CHECK-GNU-EC: .ascii " -export:#h,EXPORTAS,h
+; CHECK-GNU-NOT-EC: -export:i
+; CHECK-GNU-NOT-EC: -export:#i,EXPORTAS,i
+; CHECK-GNU-EC: .ascii " -export:#j,EXPORTAS,j"
+; CHECK-GNU-EC: .ascii " -export:#k,EXPORTAS,k"
+; CHECK-GNU-EC: .ascii " -export:#l,EXPORTAS,l"
+; CHECK-GNU-EC: .ascii " -export:m,data"
+; CHECK-GNU-EC: .ascii " -export:n,data"
+; CHECK-GNU-EC: .ascii " -export:o,data"
+; CHECK-GNU-EC: .ascii " -export:p,data"
+; CHECK-GNU-EC: .ascii " -export:q,data"
+; CHECK-GNU-EC: .ascii " -export:r"
+; CHECK-GNU-EC: .ascii " -export:s"
+; CHECK-GNU-EC: .ascii " -export:t"
+; CHECK-GNU-EC: .ascii " -export:u"
+; CHECK-MSVC-NOT-EC: /EXPORT:f
+; CHECK-MSVC-NOT-EC: /EXPORT:#f,EXPORTAS,f
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#g,EXPORTAS,g"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#h,EXPORTAS,h"
+; CHECK-MSVC-NOT-EC: /EXPORT:i
+; CHECK-MSVC-NOT-EC: /EXPORT:#i,EXPORTAS,i
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#j,EXPORTAS,j"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#k,EXPORTAS,k"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#l,EXPORTAS,l"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:m,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:n,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:o,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:p,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:q,DATA"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:r"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:s"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:t"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:u"

efriedma-quic · 2024-02-15T23:53:00Z

llvm/test/CodeGen/AArch64/dllexport.ll

+; CHECK-MSVC-EC: .ascii "  /EXPORT:#g,EXPORTAS,g"
+; CHECK-MSVC-EC: .ascii "  /EXPORT:#h,EXPORTAS,h"
+; CHECK-MSVC-NOT-EC: /EXPORT:i
+; CHECK-MSVC-NOT-EC: /EXPORT:#i,EXPORTAS,i


I don't think "CHECK-MSVC-NOT-EC" will be parsed by any of the FileCheck commands?

Ah, right. I pushed a fix, thanks.

efriedma-quic · 2024-02-16T00:55:35Z

I'm a bit concerned about the interaction between emitLinkerFlagsForGlobalCOFF() and the EC lowering pass.

It looks like emitLinkerFlagsForGlobalCOFF() is used by LTO to compute this data in some (?) cases. This is before we'd normally do EC lowering. But the code generator calls it late, after EC lowering. So there's going to be some inconsistency there.

That said, I'm not really sure what the linker is actually doing with the flags before LTO. And I guess supporting LTO isn't really a high priority at the moment. Maybe worth leaving a FIXME, at least.

cjacek · 2024-02-20T00:24:49Z

I can reproduce the inconsistency. In fact, it's already there: for LTO we use demangled names, while for the regular compilation we use mangled ones. In practice, this is fine with the linker as it will resolve demangled name from generated aliases, but it differs from the regular build.

I'm not sure what would be the best way to use EXPORTAS in that case. I was thinking about predicting that demangled functions will get mangled later anyway and mangling them using getArm64ECMangledFunctionName here anyway. Doing that in emitLinkerFlagsForGlobalCOFF somewhat works, but maybe this would be better done in getNameWithPrefix to cover other cases.

BTW, simple LTO cases seem to work with my WIP branch implementing ARM64EC in LLD (https://github.com/cjacek/llvm-project/commits/arm64ec). Hacking use of EXPORTAS as I described above actually makes it not work, because then linker doesn't know that bitcode will eventually expose ARM64EC symbols. This seems to be a more general problem, I didn't really look deeper at it yet as I'd like to get basics right first. It's likely that the fix should be in LLD, but possibly not only.

I think that a FIXME comment is best for now, I will add one.

efriedma-quic

LGTM

hokein · 2024-03-31T10:00:26Z

llvm/lib/IR/Mangler.cpp

@@ -18,8 +18,10 @@
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Object/COFF.h"


(this comes up when I tried to fix the upstream bazel build failure)

We seem to introduce a cycle dependency here, in bazel config, we have

llvm:Core target which includes all lib/IR/* files

llvm:Object target depends on llvm:Core

This change makes llvm:Core depends on llvm:Object. Can we remove this dependency? We only need object::getArm64ECDemangledFunctionName in this file, is it possible (and sensible) to move this inline getArm64ECDemangledFunctionName from COFF.h to Mangler.h or a new file?

I didn't know that inline functions are a problem, sorry. I created #87191 moving those helpers to Mangler.h.

cjacek requested review from mstorsjo, dpaoliello and efriedma-quic February 15, 2024 23:47

llvmbot added backend:AArch64 llvm:ir labels Feb 15, 2024

cjacek force-pushed the dllexport branch from 7746a01 to 3f59514 Compare February 15, 2024 23:48

efriedma-quic reviewed Feb 15, 2024

View reviewed changes

cjacek mentioned this pull request Mar 29, 2024

[LLD][COFF] Demangle ARM64EC export names. #87068

Merged

[IR] Use EXPORTAS for ARM64EC mangled symbols with dllexport attribute.

c7a3469

cjacek force-pushed the dllexport branch from 7b06c51 to c7a3469 Compare March 29, 2024 14:04

efriedma-quic approved these changes Mar 29, 2024

View reviewed changes

cjacek merged commit 799e1d6 into llvm:main Mar 30, 2024

cjacek deleted the dllexport branch March 30, 2024 15:48

hokein reviewed Mar 31, 2024

View reviewed changes

cjacek mentioned this pull request Mar 31, 2024

[IR][Object][NFC] Move ARM64EC name mangling helpers to Mangler.h. #87191

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[IR] Use EXPORTAS for ARM64EC mangled symbols with dllexport attribute. #81940

[IR] Use EXPORTAS for ARM64EC mangled symbols with dllexport attribute. #81940

Uh oh!

cjacek commented Feb 15, 2024

Uh oh!

llvmbot commented Feb 15, 2024

Uh oh!

llvmbot commented Feb 15, 2024

Uh oh!

efriedma-quic Feb 15, 2024

Uh oh!

cjacek Feb 16, 2024

Uh oh!

efriedma-quic commented Feb 16, 2024

Uh oh!

cjacek commented Feb 20, 2024

Uh oh!

efriedma-quic left a comment

Uh oh!

hokein Mar 31, 2024

Uh oh!

cjacek Mar 31, 2024

Uh oh!

Uh oh!

[IR] Use EXPORTAS for ARM64EC mangled symbols with dllexport attribute. #81940

[IR] Use EXPORTAS for ARM64EC mangled symbols with dllexport attribute. #81940

Uh oh!

Conversation

cjacek commented Feb 15, 2024

Uh oh!

llvmbot commented Feb 15, 2024

Uh oh!

llvmbot commented Feb 15, 2024

Uh oh!

efriedma-quic Feb 15, 2024

Choose a reason for hiding this comment

Uh oh!

cjacek Feb 16, 2024

Choose a reason for hiding this comment

Uh oh!

efriedma-quic commented Feb 16, 2024

Uh oh!

cjacek commented Feb 20, 2024

Uh oh!

efriedma-quic left a comment

Choose a reason for hiding this comment

Uh oh!

hokein Mar 31, 2024

Choose a reason for hiding this comment

Uh oh!

cjacek Mar 31, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!