[PATCH] D60161: Expose non-trivial struct helpers for Swift.
rjmccall closed this revision. rjmccall added a comment. Committed as r358132. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
allevato added a comment. Oh, and in case I need to mention this (I don't contribute to llvm/clang frequently), I don't have commit access so I'll need someone else to merge it in. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM, thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
allevato marked 2 inline comments as done. allevato added a comment. Thanks for the review! This is ready to go on my end if there aren't any other comments. Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:845 + // actually use them (it overwrites them with the addresses of the arguments + // of the created function). + return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments, ahatanak wrote: > I think `getFunction` shouldn't require passing addresses, but that's not > this patch's fault. I'll see if I can remove the parameter. Yeah, this hack bummed me out and I tried cleaning up the other related functions to have them not reuse the array in this way, but the way std::array and the template arg size_t N are currently being used throughout made those attempts ripple through in ways that would have required a deeper refactor. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
allevato updated this revision to Diff 194530. allevato added a comment. - Rename generate* to get* and cleanup param names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 Files: clang/include/clang/CodeGen/CodeGenABITypes.h clang/lib/CodeGen/CGNonTrivialStruct.cpp Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp === --- clang/lib/CodeGen/CGNonTrivialStruct.cpp +++ clang/lib/CodeGen/CGNonTrivialStruct.cpp @@ -14,6 +14,7 @@ #include "CodeGenFunction.h" #include "CodeGenModule.h" #include "clang/AST/NonTrivialTypeVisitor.h" +#include "clang/CodeGen/CodeGenABITypes.h" #include "llvm/Support/ScopedPrinter.h" #include @@ -822,6 +823,29 @@ Gen.callFunc(FuncName, QT, Addrs, CGF); } +template std::array createNullAddressArray(); + +template <> std::array createNullAddressArray() { + return std::array({Address(nullptr, CharUnits::Zero())}); +} + +template <> std::array createNullAddressArray() { + return std::array({Address(nullptr, CharUnits::Zero()), + Address(nullptr, CharUnits::Zero())}); +} + +template +static llvm::Function * +getSpecialFunction(G &, StringRef FuncName, QualType QT, bool IsVolatile, + std::array Alignments, CodeGenModule ) { + QT = IsVolatile ? QT.withVolatile() : QT; + // The following call requires an array of addresses as arguments, but doesn't + // actually use them (it overwrites them with the addresses of the arguments + // of the created function). + return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments, + CGM); +} + // Functions to emit calls to the special functions of a non-trivial C struct. void CodeGenFunction::callCStructDefaultConstructor(LValue Dst) { bool IsVolatile = Dst.isVolatile(); @@ -907,3 +931,69 @@ callSpecialFunction(GenMoveAssignment(getContext()), FuncName, QT, IsVolatile, *this, std::array({{DstPtr, SrcPtr}})); } + +llvm::Function *clang::CodeGen::getNonTrivialCStructDefaultConstructor( +CodeGenModule , CharUnits DstAlignment, bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenDefaultInitializeFuncName GenName(DstAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return getSpecialFunction(GenDefaultInitialize(Ctx), FuncName, QT, IsVolatile, +std::array({{DstAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::getNonTrivialCStructCopyConstructor( +CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment, +bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenBinaryFuncName GenName("__copy_constructor_", DstAlignment, + SrcAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return getSpecialFunction( + GenCopyConstructor(Ctx), FuncName, QT, IsVolatile, + std::array({{DstAlignment, SrcAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::getNonTrivialCStructMoveConstructor( +CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment, +bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenBinaryFuncName GenName("__move_constructor_", DstAlignment, + SrcAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return getSpecialFunction( + GenMoveConstructor(Ctx), FuncName, QT, IsVolatile, + std::array({{DstAlignment, SrcAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::getNonTrivialCStructCopyAssignmentOperator( +CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment, +bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenBinaryFuncName GenName("__copy_assignment_", DstAlignment, + SrcAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return getSpecialFunction( + GenCopyAssignment(Ctx), FuncName, QT, IsVolatile, + std::array({{DstAlignment, SrcAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::getNonTrivialCStructMoveAssignmentOperator( +CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment, +bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenBinaryFuncName GenName("__move_assignment_", DstAlignment, + SrcAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return getSpecialFunction( + GenMoveAssignment(Ctx), FuncName, QT, IsVolatile, + std::array({{DstAlignment, SrcAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::getNonTrivialCStructDestructor( +CodeGenModule , CharUnits DstAlignment, bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenDestructorFuncName GenName("__destructor_", DstAlignment, Ctx); + std::string
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
ahatanak added a comment. Just one minor comment, but otherwise the patch looks good to me. Comment at: clang/include/clang/CodeGen/CodeGenABITypes.h:92 +llvm::Function *generateNonTrivialCStructDefaultConstructor(CodeGenModule , +CharUnits Alignment, +bool IsVolatile, The parameter name is different from the one that is used in the function definition (`CharUnits DstAlignment`). Ditto for `generateNonTrivialCStructDestructor`. Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:845 + // actually use them (it overwrites them with the addresses of the arguments + // of the created function). + return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments, I think `getFunction` shouldn't require passing addresses, but that's not this patch's fault. I'll see if I can remove the parameter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
rjmccall added a reviewer: ahatanak. rjmccall added inline comments. Herald added a subscriber: dexonsmith. Comment at: clang/include/clang/CodeGen/CodeGenABITypes.h:140 +QualType QT); + } // end namespace CodeGen Would you mind calling these `get...` rather than `generate...`? It'd make it clearer that they will re-use an existing definition if possible. Otherwise this looks great, thanks! Akira, any comments? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
allevato updated this revision to Diff 194379. allevato added a comment. - Apply review changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 Files: clang/include/clang/CodeGen/CodeGenABITypes.h clang/lib/CodeGen/CGNonTrivialStruct.cpp Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp === --- clang/lib/CodeGen/CGNonTrivialStruct.cpp +++ clang/lib/CodeGen/CGNonTrivialStruct.cpp @@ -14,6 +14,7 @@ #include "CodeGenFunction.h" #include "CodeGenModule.h" #include "clang/AST/NonTrivialTypeVisitor.h" +#include "clang/CodeGen/CodeGenABITypes.h" #include "llvm/Support/ScopedPrinter.h" #include @@ -822,6 +823,30 @@ Gen.callFunc(FuncName, QT, Addrs, CGF); } +template std::array createNullAddressArray(); + +template <> std::array createNullAddressArray() { + return std::array({Address(nullptr, CharUnits::Zero())}); +} + +template <> std::array createNullAddressArray() { + return std::array({Address(nullptr, CharUnits::Zero()), + Address(nullptr, CharUnits::Zero())}); +} + +template +static llvm::Function * +generateSpecialFunction(G &, StringRef FuncName, QualType QT, +bool IsVolatile, std::array Alignments, +CodeGenModule ) { + QT = IsVolatile ? QT.withVolatile() : QT; + // The following call requires an array of addresses as arguments, but doesn't + // actually use them (it overwrites them with the addresses of the arguments + // of the created function). + return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments, + CGM); +} + // Functions to emit calls to the special functions of a non-trivial C struct. void CodeGenFunction::callCStructDefaultConstructor(LValue Dst) { bool IsVolatile = Dst.isVolatile(); @@ -907,3 +932,70 @@ callSpecialFunction(GenMoveAssignment(getContext()), FuncName, QT, IsVolatile, *this, std::array({{DstPtr, SrcPtr}})); } + +llvm::Function *clang::CodeGen::generateNonTrivialCStructDefaultConstructor( +CodeGenModule , CharUnits DstAlignment, bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenDefaultInitializeFuncName GenName(DstAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return generateSpecialFunction( + GenDefaultInitialize(Ctx), FuncName, QT, IsVolatile, + std::array({{DstAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::generateNonTrivialCStructCopyConstructor( +CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment, +bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenBinaryFuncName GenName("__copy_constructor_", DstAlignment, + SrcAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return generateSpecialFunction( + GenCopyConstructor(Ctx), FuncName, QT, IsVolatile, + std::array({{DstAlignment, SrcAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::generateNonTrivialCStructMoveConstructor( +CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment, +bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenBinaryFuncName GenName("__move_constructor_", DstAlignment, + SrcAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return generateSpecialFunction( + GenMoveConstructor(Ctx), FuncName, QT, IsVolatile, + std::array({{DstAlignment, SrcAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::generateNonTrivialCStructCopyAssignmentOperator( +CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment, +bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenBinaryFuncName GenName("__copy_assignment_", DstAlignment, + SrcAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return generateSpecialFunction( + GenCopyAssignment(Ctx), FuncName, QT, IsVolatile, + std::array({{DstAlignment, SrcAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::generateNonTrivialCStructMoveAssignmentOperator( +CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment, +bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenBinaryFuncName GenName("__move_assignment_", DstAlignment, + SrcAlignment, Ctx); + std::string FuncName = GenName.getName(QT, IsVolatile); + return generateSpecialFunction( + GenMoveAssignment(Ctx), FuncName, QT, IsVolatile, + std::array({{DstAlignment, SrcAlignment}}), CGM); +} + +llvm::Function *clang::CodeGen::generateNonTrivialCStructDestructor( +CodeGenModule , CharUnits Alignment, bool IsVolatile, QualType QT) { + ASTContext = CGM.getContext(); + GenDestructorFuncName
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
allevato added inline comments. Comment at: clang/include/clang/CodeGen/CodeGenABITypes.h:93 +llvm::BasicBlock::iterator InsertPoint, llvm::Value *Dst, +CharUnits Alignment, QualType QT); + rjmccall wrote: > Hmm. I think it might be better to just have this return a function pointer; > emitting a call in a different frontend's function can be pretty fraught if > that frontend wants to handle e.g. unwinding. The interface should document > the ABI of the returned function pointer. Callers might be able to take > advantage of a function pointer in different ways, e.g. by using it directly > in Swift's value-witness tables on targets where the calling conventions > align. > > In the long run, I'd definitely like to make this interface capable of doing > some limited code-generation tasks, e.g. to load or store bit-fields, but I > suspect that we'll need to think about that a bit more carefully and that we > might want to avoid anything that involves a call. That makes sense—getting this working was a bit tricky and passing the BB and insert point didn't exactly seem like the cleanest approach. I'll work on the alternate approach you suggested and circle back around when I have it working with the corresponding Swift changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
rjmccall added inline comments. Comment at: clang/include/clang/CodeGen/CodeGenABITypes.h:93 +llvm::BasicBlock::iterator InsertPoint, llvm::Value *Dst, +CharUnits Alignment, QualType QT); + Hmm. I think it might be better to just have this return a function pointer; emitting a call in a different frontend's function can be pretty fraught if that frontend wants to handle e.g. unwinding. The interface should document the ABI of the returned function pointer. Callers might be able to take advantage of a function pointer in different ways, e.g. by using it directly in Swift's value-witness tables on targets where the calling conventions align. In the long run, I'd definitely like to make this interface capable of doing some limited code-generation tasks, e.g. to load or store bit-fields, but I suspect that we'll need to think about that a bit more carefully and that we might want to avoid anything that involves a call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60161/new/ https://reviews.llvm.org/D60161 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D60161: Expose non-trivial struct helpers for Swift.
allevato created this revision. allevato added a reviewer: rjmccall. Herald added subscribers: cfe-commits, jdoerfert. Herald added a project: clang. This change exposes the helper functions that generate ctors/dtors for non-trivial structs so that they can be called from the Swift compiler (see https://github.com/apple/swift/pull/23405) to enable it to import such C records. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D60161 Files: clang/include/clang/CodeGen/CodeGenABITypes.h clang/lib/CodeGen/CGNonTrivialStruct.cpp Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp === --- clang/lib/CodeGen/CGNonTrivialStruct.cpp +++ clang/lib/CodeGen/CGNonTrivialStruct.cpp @@ -14,6 +14,7 @@ #include "CodeGenFunction.h" #include "CodeGenModule.h" #include "clang/AST/NonTrivialTypeVisitor.h" +#include "clang/CodeGen/CodeGenABITypes.h" #include "llvm/Support/ScopedPrinter.h" #include @@ -907,3 +908,71 @@ callSpecialFunction(GenMoveAssignment(getContext()), FuncName, QT, IsVolatile, *this, std::array({{DstPtr, SrcPtr}})); } + +void clang::CodeGen::callNonTrivialCStructDefaultConstructor( +CodeGenModule , llvm::BasicBlock *BB, +llvm::BasicBlock::iterator InsertPoint, llvm::Value *Dst, +CharUnits DstAlignment, QualType QT) { + CodeGenFunction CGF(CGM, true); + CGF.Builder.SetInsertPoint(BB, InsertPoint); + LValue DstLValue = CGF.MakeAddrLValue(Dst, QT, DstAlignment); + CGF.callCStructDefaultConstructor(DstLValue); +} + +void clang::CodeGen::callNonTrivialCStructCopyConstructor( +CodeGenModule , llvm::BasicBlock *BB, +llvm::BasicBlock::iterator InsertPoint, llvm::Value *Dst, +CharUnits DstAlignment, llvm::Value *Src, CharUnits SrcAlignment, +QualType QT) { + CodeGenFunction CGF(CGM, true); + CGF.Builder.SetInsertPoint(BB, InsertPoint); + LValue DstLValue = CGF.MakeAddrLValue(Dst, QT, DstAlignment); + LValue SrcLValue = CGF.MakeAddrLValue(Src, QT, SrcAlignment); + CGF.callCStructCopyConstructor(DstLValue, SrcLValue); +} + +void clang::CodeGen::callNonTrivialCStructMoveConstructor( +CodeGenModule , llvm::BasicBlock *BB, +llvm::BasicBlock::iterator InsertPoint, llvm::Value *Dst, +CharUnits DstAlignment, llvm::Value *Src, CharUnits SrcAlignment, +QualType QT) { + CodeGenFunction CGF(CGM, true); + CGF.Builder.SetInsertPoint(BB, InsertPoint); + LValue DstLValue = CGF.MakeAddrLValue(Dst, QT, DstAlignment); + LValue SrcLValue = CGF.MakeAddrLValue(Src, QT, SrcAlignment); + CGF.callCStructMoveConstructor(DstLValue, SrcLValue); +} + +void clang::CodeGen::callNonTrivialCStructCopyAssignmentOperator( +CodeGenModule , llvm::BasicBlock *BB, +llvm::BasicBlock::iterator InsertPoint, llvm::Value *Dst, +CharUnits DstAlignment, llvm::Value *Src, CharUnits SrcAlignment, +QualType QT) { + CodeGenFunction CGF(CGM, true); + CGF.Builder.SetInsertPoint(BB, InsertPoint); + LValue DstLValue = CGF.MakeAddrLValue(Dst, QT, DstAlignment); + LValue SrcLValue = CGF.MakeAddrLValue(Src, QT, SrcAlignment); + CGF.callCStructCopyAssignmentOperator(DstLValue, SrcLValue); +} + +void clang::CodeGen::callNonTrivialCStructMoveAssignmentOperator( +CodeGenModule , llvm::BasicBlock *BB, +llvm::BasicBlock::iterator InsertPoint, llvm::Value *Dst, +CharUnits DstAlignment, llvm::Value *Src, CharUnits SrcAlignment, +QualType QT) { + CodeGenFunction CGF(CGM, true); + CGF.Builder.SetInsertPoint(BB, InsertPoint); + LValue DstLValue = CGF.MakeAddrLValue(Dst, QT, DstAlignment); + LValue SrcLValue = CGF.MakeAddrLValue(Src, QT, SrcAlignment); + CGF.callCStructMoveAssignmentOperator(DstLValue, SrcLValue); +} + +void clang::CodeGen::callNonTrivialCStructDestructor( +CodeGenModule , llvm::BasicBlock *BB, +llvm::BasicBlock::iterator InsertPoint, llvm::Value *Dst, +CharUnits Alignment, QualType QT) { + CodeGenFunction CGF(CGM, true); + CGF.Builder.SetInsertPoint(BB, InsertPoint); + LValue lvalue = CGF.MakeAddrLValue(Dst, QT, Alignment); + CGF.callCStructDestructor(lvalue); +} Index: clang/include/clang/CodeGen/CodeGenABITypes.h === --- clang/include/clang/CodeGen/CodeGenABITypes.h +++ clang/include/clang/CodeGen/CodeGenABITypes.h @@ -26,12 +26,14 @@ #include "clang/AST/CanonicalType.h" #include "clang/AST/Type.h" #include "clang/CodeGen/CGFunctionInfo.h" +#include "llvm/IR/BasicBlock.h" namespace llvm { class DataLayout; class Module; class FunctionType; class Type; + class Value; } namespace clang { @@ -83,6 +85,52 @@ unsigned getLLVMFieldNumber(CodeGenModule , const RecordDecl *RD, const FieldDecl *FD); +/// Generates a call to the default constructor for a C struct with +/// non-trivially copyable fields. +void callNonTrivialCStructDefaultConstructor( +CodeGenModule , llvm::BasicBlock *BB, +llvm::BasicBlock::iterator