Skip to content

[WIP] stdlib: add support for AAPCS64 variadics #12623

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[WIP] stdlib: add support for AAPCS64 variadics #12623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ getSwiftStdlibType(const clang::TypedefNameDecl *D,
return std::make_pair(Type(), "");

// Check other expected properties of the C type.
switch(CTypeKind) {
switch (CTypeKind) {
case MappedCTypeKind::UnsignedInt:
if (!ClangType->isUnsignedIntegerType())
return std::make_pair(Type(), "");
Expand Down Expand Up @@ -273,8 +273,6 @@ getSwiftStdlibType(const clang::TypedefNameDecl *D,
break;

case MappedCTypeKind::VaList:
if (ClangTypeSize != ClangCtx.getTypeSize(ClangCtx.VoidPtrTy))
return std::make_pair(Type(), "");
break;

case MappedCTypeKind::ObjCBool:
Expand Down Expand Up @@ -334,6 +332,20 @@ getSwiftStdlibType(const clang::TypedefNameDecl *D,
*IsError = true;
return std::make_pair(Type(), "");
}

if (CTypeKind == MappedCTypeKind::VaList) {
const llvm::Triple &T = ClangCtx.getTargetInfo().getTriple();
// Darwin AArch64 uses void * for the va_list type, while AAPCS64 uses a
// custom type. Once the TODO below is addressed, this check should not be
// needed.
bool IsAAPCS64 = T.getArch() == llvm::Triple::AArch64 && !T.isOSDarwin();
// TODO compare this against the size of the SwiftType rather than the
// void * type.
if (!IsAAPCS64 &&
(ClangTypeSize != ClangCtx.getTypeSize(ClangCtx.VoidPtrTy)))
Copy link
Member Author

Choose a reason for hiding this comment

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

@jrose-apple I couldn't see the equivalent of getTypeSize in the swift ASTContext, perhaps I missed something? Is there an easy way to address that TODO?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not really. Swift doesn't bother measuring sizes until IR generation time, and even then you won't be able to do it under certain conditions. I think it's okay to just explicitly leave AAPCS64 out of the impedance check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, Ill probably move this back into the switch then

return std::make_pair(Type(), "");
}

return std::make_pair(SwiftType, SwiftTypeName);
}

Expand Down
25 changes: 25 additions & 0 deletions stdlib/public/core/CTypes.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,30 @@ extension OpaquePointer : Equatable {
}

/// The corresponding Swift type to `va_list` in imported C APIs.
#if arch(arm64) && !(os(macOS) || os(iOS) || os(tvOS) || os(watchOS))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment here describing this as the AAPCS64 case.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure


// non-Darwin AAPCS64 ABI
@_fixed_layout
public struct CVaListPointer {
@_versioned // FIXME(sil-serialize-all)
internal var value: (__stack: UnsafeMutablePointer<Int>?,
__gr_top: UnsafeMutablePointer<Int>?,
__vr_top: UnsafeMutablePointer<Int>?,
__gr_off: Int32,
__vr_off: Int32)

@_inlineable // FIXME(sil-serialize-all)
public // @testable
init(__stack: UnsafeMutablePointer<Int>?,
__gr_top: UnsafeMutablePointer<Int>?,
__vr_top: UnsafeMutablePointer<Int>?,
__gr_off: Int32,
__vr_off: Int32) {
value = (__stack, __gr_top, __vr_top, __gr_off, __vr_off)
}
}

#else
@_fixed_layout
public struct CVaListPointer {
@_versioned // FIXME(sil-serialize-all)
Expand All @@ -220,6 +244,7 @@ extension CVaListPointer : CustomDebugStringConvertible {
return value.debugDescription
}
}
#endif
Copy link
Contributor

@gparker42 gparker42 Oct 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This #endif should be up a few lines. The CustomDebugStringConvertible extension should be valid for all definitions.

…and if it isn't valid, you should add a valid one for the AAPCS version.

Copy link
Member Author

@compnerd compnerd Oct 26, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The value type is different in all cases. In fact, for the AAPCS64 case, its a tuple, rather than an UnsafeMutablePointer. So this doesn't apply in all cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

Saleem and I briefly talked about this at the LLVM dev meeting, and I told him to go ahead without it. I'm not sure we have a good debug description for a struct-based va_list that's any better than just showing the children (the default behavior of String(reflecting:)).


@_versioned
@_inlineable
Expand Down
35 changes: 34 additions & 1 deletion stdlib/public/core/VarArgs.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,40 @@ extension Double : _CVarArgPassedAsDouble, _CVarArgAligned {
}
}

#if arch(x86_64)
#if arch(arm64) && !(os(macOS) || os(iOS) || os(tvOS) || os(watchOS))

@_fixed_layout // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
final internal class _VaListBuilder {
@_versioned // FIXME(sil-serialize-all)
internal var __stack: ContiguousArray<Int>

@_inlineable // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
internal init() {
__stack = ContiguousArray(repeating: 0, count: 0)
}

@_inlineable // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
deinit {}

@_inlineable // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
internal func append(_ arg: CVarArg) {
}

@_inlineable // FIXME(sil-serialize-all)
@_versioned // FIXME(sil-serialize-all)
internal func va_list() -> CVaListPointer {
let __gr_top = UnsafeMutablePointer<Int>(bitPattern: 0)
let __vr_top = UnsafeMutablePointer<Int>(bitPattern: 0)
return CVaListPointer(__stack: __stack._baseAddress, __gr_top: __gr_top,
__vr_top: __vr_top, __gr_off: 0, __vr_off: 0)
}
}

#elseif arch(x86_64)

/// An object that can manage the lifetime of storage backing a
/// `CVaListPointer`.
Expand Down