This is an archive of the discontinued LLVM Phabricator instance.

Unblock Swift Calling Convention Mangling on Windows
AbandonedPublic

Authored by troughton on Dec 6 2017, 4:01 PM.

Details

Reviewers
rjmccall
compnerd
rnk
majnemer
Group Reviewers
Restricted Project
Summary

Currently, when Clang is trying to mangle a function that uses the Swift calling convention, it crashes in an llvm_unreachable statement. This blocks the Swift calling convention from being used on Windows.

Following discussion on the swift-dev mailing list and on a pull request for swift-clang, unblock support for the Swift calling convention on targets using Microsoft mangling by mangling the name as per the __cdecl calling convention.

Also in this PR, replace llvm_unreachable with an error diagnostic since this code path can be hit.

Diff Detail

Repository
rC Clang

Event Timeline

troughton created this revision.Dec 6 2017, 4:01 PM
smeenai added a subscriber: smeenai.Dec 6 2017, 4:41 PM

Patch is missing context.

lib/AST/MicrosoftMangle.cpp
2133

You still need the default label, right?

troughton updated this revision to Diff 125847.Dec 6 2017, 4:54 PM

Add back a mistakenly removed default case.

troughton edited the summary of this revision. (Show Details)Dec 6 2017, 6:21 PM
troughton edited the summary of this revision. (Show Details)
rjmccall edited edge metadata.

Reid, David, do you have a recommendation about the right way to support non-MS-supported CCs here?

rnk added a comment.Dec 14 2017, 1:48 PM

@compnerd's suggestion is still a decent one: https://reviews.llvm.org/D31372

Which, I think is just adding something wacky like @swiftcc@__Swift@@ which would demangle as __Swift::swiftcc if the demangler expected an NNS there. Of course, it doesn't, so it won't demangle, but at least you can overload between cdecl and swiftcc then.

lib/AST/MicrosoftMangle.cpp
2133

Surely we can emit a real custom diagnostic, similar to CodeGenModule::ErrorUnsupported.

troughton marked an inline comment as done.Dec 15 2017, 12:59 AM

Which, I think is just adding something wacky like @swiftcc@__Swift@@ which would demangle as __Swift::swiftcc if the demangler expected an NNS there. Of course, it doesn't, so it won't demangle, but at least you can overload between cdecl and swiftcc then.

Could we do a combination of these? Use the A prefix for the CC slot but then append the @swiftcc@__Swift@@ at the end of the qualified name? That way you could still distinguish a SwiftCC function and have it correctly demangle with existing tools.

As for emitting a proper diagnostic: we definitely can do something better here. I wasn't sure what to base it off so went with the simplest option, but given that CodeGenModule::ErrorUnsupported example I'll take a look and try to implement something similar.

lib/AST/MicrosoftMangle.cpp
2133

Oops, yes. Thanks. I've added it back.

D42768 adds the __swift_cc in the right location and detangles correctly with undname as well.

troughton abandoned this revision.Jan 31 2018, 4:26 PM
troughton marked an inline comment as done.