From f1788639c5f62beedd11adc40c144afff46b2a45 Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 28 Nov 2016 22:18:30 +0000 Subject: [PATCH] Hide the result of building a constant initializer. NFC. llvm-svn: 288080 --- clang/lib/CodeGen/CGObjCGNU.cpp | 30 ++++---- clang/lib/CodeGen/CGOpenMPRuntime.cpp | 2 +- clang/lib/CodeGen/CodeGenModule.cpp | 2 +- clang/lib/CodeGen/ConstantBuilder.h | 105 ++++++++++++++++++-------- 4 files changed, 91 insertions(+), 48 deletions(-) diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp index 1f64b8ae268d..dd3f2028d3a2 100644 --- a/clang/lib/CodeGen/CGObjCGNU.cpp +++ b/clang/lib/CodeGen/CGObjCGNU.cpp @@ -1549,7 +1549,7 @@ GenerateMethodList(StringRef ClassName, Methods.add( llvm::ConstantStruct::get(ObjCMethodTy, {C, MethodTypes[i], Method})); } - MethodList.add(Methods.finish()); + Methods.finishAndAddTo(MethodList); // Create an instance of the structure return MethodList.finishAndCreateGlobal(".objc_method_list", @@ -1584,9 +1584,9 @@ GenerateIvarList(ArrayRef IvarNames, Ivar.add(IvarNames[i]); Ivar.add(IvarTypes[i]); Ivar.add(IvarOffsets[i]); - Ivars.add(Ivar.finish()); + Ivar.finishAndAddTo(Ivars); } - IvarList.add(Ivars.finish()); + Ivars.finishAndAddTo(IvarList); // Create an instance of the structure return IvarList.finishAndCreateGlobal(".objc_ivar_list", @@ -1720,9 +1720,9 @@ GenerateProtocolMethodList(ArrayRef MethodNames, auto Method = Methods.beginStruct(ObjCMethodDescTy); Method.add(MethodNames[i]); Method.add(MethodTypes[i]); - Methods.add(Method.finish()); + Method.finishAndAddTo(Methods); } - MethodList.add(Methods.finish()); + Methods.finishAndAddTo(MethodList); return MethodList.finishAndCreateGlobal(".objc_method_list", CGM.getPointerAlign()); } @@ -1751,7 +1751,7 @@ CGObjCGNU::GenerateProtocolList(ArrayRef Protocols) { PtrToInt8Ty); Elements.add(Ptr); } - ProtocolList.add(Elements.finish()); + Elements.finishAndAddTo(ProtocolList); return ProtocolList.finishAndCreateGlobal(".objc_protocol_list", CGM.getPointerAlign()); } @@ -1914,15 +1914,15 @@ void CGObjCGNU::GenerateProtocol(const ObjCProtocolDecl *PD) { fields.add(NULLPtr); } - propertiesArray.add(fields.finish()); + fields.finishAndAddTo(propertiesArray); } - reqPropertiesList.add(reqPropertiesArray.finish()); + reqPropertiesArray.finishAndAddTo(reqPropertiesList); PropertyList = reqPropertiesList.finishAndCreateGlobal(".objc_property_list", CGM.getPointerAlign()); - optPropertiesList.add(optPropertiesArray.finish()); + optPropertiesArray.finishAndAddTo(optPropertiesList); OptionalPropertyList = optPropertiesList.finishAndCreateGlobal(".objc_property_list", CGM.getPointerAlign()); @@ -1982,7 +1982,7 @@ void CGObjCGNU::GenerateProtocolHolderCategory() { PtrTy); ProtocolElements.add(Ptr); } - ProtocolList.add(ProtocolElements.finish()); + ProtocolElements.finishAndAddTo(ProtocolList); Elements.add(llvm::ConstantExpr::getBitCast( ProtocolList.finishAndCreateGlobal(".objc_protocol_list", CGM.getPointerAlign()), @@ -2029,7 +2029,7 @@ llvm::Constant *CGObjCGNU::MakeBitField(ArrayRef bits) { fields.addInt(Int32Ty, values.size()); auto array = fields.beginArray(); for (auto v : values) array.add(v); - fields.add(array.finish()); + array.finishAndAddTo(fields); llvm::Constant *GS = fields.finishAndCreateGlobal("", CharUnits::fromQuantity(4)); @@ -2153,9 +2153,9 @@ llvm::Constant *CGObjCGNU::GeneratePropertyList(const ObjCImplementationDecl *OI fields.add(NULLPtr); fields.add(NULLPtr); } - properties.add(fields.finish()); + fields.finishAndAddTo(properties); } - propertyList.add(properties.finish()); + properties.finishAndAddTo(propertyList); return propertyList.finishAndCreateGlobal(".objc_property_list", CGM.getPointerAlign()); @@ -2483,7 +2483,7 @@ llvm::Function *CGObjCGNU::ModuleInitFunction() { auto SelStruct = Selectors.beginStruct(SelStructTy); SelStruct.add(SelName); SelStruct.add(SelectorTypeEncoding); - Selectors.add(SelStruct.finish()); + SelStruct.finishAndAddTo(Selectors); // Store the selector alias for later replacement SelectorAliases.push_back(i->second); @@ -2498,7 +2498,7 @@ llvm::Function *CGObjCGNU::ModuleInitFunction() { auto SelStruct = Selectors.beginStruct(SelStructTy); SelStruct.add(NULLPtr); SelStruct.add(NULLPtr); - Selectors.add(SelStruct.finish()); + SelStruct.finishAndAddTo(Selectors); } // Number of static selectors diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp index b88c6b1185c8..c03e44994fb9 100644 --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -2832,7 +2832,7 @@ CGOpenMPRuntime::createOffloadingBinaryDescriptorRegistration() { Dev.add(ImgEnd); Dev.add(HostEntriesBegin); Dev.add(HostEntriesEnd); - DeviceImagesEntries.add(Dev.finish()); + Dev.finishAndAddTo(DeviceImagesEntries); } // Create device images global array. diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 0502709389b4..9ea11b8e1c47 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -753,7 +753,7 @@ void CodeGenModule::EmitCtorList(CtorList &Fns, const char *GlobalName) { ctor.add(llvm::ConstantExpr::getBitCast(I.AssociatedData, VoidPtrTy)); else ctor.addNullPointer(VoidPtrTy); - ctors.add(ctor.finish()); + ctor.finishAndAddTo(ctors); } auto list = diff --git a/clang/lib/CodeGen/ConstantBuilder.h b/clang/lib/CodeGen/ConstantBuilder.h index e6aa34e4f906..918a48d97397 100644 --- a/clang/lib/CodeGen/ConstantBuilder.h +++ b/clang/lib/CodeGen/ConstantBuilder.h @@ -58,10 +58,10 @@ public: assert(Buffer.empty() && "didn't claim all values out of buffer"); } - class AggregateBuilder { + class AggregateBuilderBase { protected: ConstantInitBuilder &Builder; - AggregateBuilder *Parent; + AggregateBuilderBase *Parent; size_t Begin; bool Finished = false; bool Frozen = false; @@ -74,8 +74,8 @@ public: return Builder.Buffer; } - AggregateBuilder(ConstantInitBuilder &builder, - AggregateBuilder *parent) + AggregateBuilderBase(ConstantInitBuilder &builder, + AggregateBuilderBase *parent) : Builder(builder), Parent(parent), Begin(builder.Buffer.size()) { if (parent) { assert(!parent->Frozen && "parent already has child builder active"); @@ -86,7 +86,7 @@ public: } } - ~AggregateBuilder() { + ~AggregateBuilderBase() { assert(Finished && "didn't claim value from aggregate builder"); } @@ -107,17 +107,17 @@ public: public: // Not copyable. - AggregateBuilder(const AggregateBuilder &) = delete; - AggregateBuilder &operator=(const AggregateBuilder &) = delete; + AggregateBuilderBase(const AggregateBuilderBase &) = delete; + AggregateBuilderBase &operator=(const AggregateBuilderBase &) = delete; // Movable, mostly to allow returning. But we have to write this out // properly to satisfy the assert in the destructor. - AggregateBuilder(AggregateBuilder &&other) + AggregateBuilderBase(AggregateBuilderBase &&other) : Builder(other.Builder), Parent(other.Parent), Begin(other.Begin), Finished(other.Finished), Frozen(other.Frozen) { other.Finished = false; } - AggregateBuilder &operator=(AggregateBuilder &&other) = delete; + AggregateBuilderBase &operator=(AggregateBuilderBase &&other) = delete; void add(llvm::Constant *value) { assert(!Finished && "cannot add more values after finishing builder"); @@ -165,10 +165,54 @@ public: } }; + template + class AggregateBuilder : public AggregateBuilderBase { + protected: + AggregateBuilder(ConstantInitBuilder &builder, + AggregateBuilderBase *parent) + : AggregateBuilderBase(builder, parent) {} + + Impl &asImpl() { return *static_cast(this); } + + public: + /// Given that this builder was created by beginning an array or struct + /// component on the given parent builder, finish the array/struct + /// component and add it to the parent. + /// + /// It is an intentional choice that the parent is passed in explicitly + /// despite it being redundant with information already kept in the + /// builder. This aids in readability by making it easier to find the + /// places that add components to a builder, as well as "bookending" + /// the sub-builder more explicitly. + void finishAndAddTo(AggregateBuilderBase &parent) { + assert(Parent == &parent && "adding to non-parent builder"); + parent.add(asImpl().finishImpl()); + } + + /// Given that this builder was created by beginning an array or struct + /// directly on a ConstantInitBuilder, finish the array/struct and + /// create a global variable with it as the initializer. + template + llvm::GlobalVariable *finishAndCreateGlobal(As &&...args) { + assert(!Parent && "finishing non-root builder"); + return Builder.createGlobal(asImpl().finishImpl(), + std::forward(args)...); + } + + /// Given that this builder was created by beginning an array or struct + /// directly on a ConstantInitBuilder, finish the array/struct and + /// set it as the initializer of the given global variable. + void finishAndSetAsInitializer(llvm::GlobalVariable *global) { + assert(!Parent && "finishing non-root builder"); + return Builder.setGlobalInitializer(global, asImpl().finishImpl()); + } + }; + ConstantArrayBuilder beginArray(llvm::Type *eltTy = nullptr); ConstantStructBuilder beginStruct(llvm::StructType *structTy = nullptr); +private: llvm::GlobalVariable *createGlobal(llvm::Constant *initializer, StringRef name, CharUnits alignment, @@ -188,15 +232,22 @@ public: GV->setAlignment(alignment.getQuantity()); return GV; } + + void setGlobalInitializer(llvm::GlobalVariable *GV, + llvm::Constant *initializer) { + GV->setInitializer(initializer); + } }; /// A helper class of ConstantInitBuilder, used for building constant /// array initializers. -class ConstantArrayBuilder : public ConstantInitBuilder::AggregateBuilder { +class ConstantArrayBuilder + : public ConstantInitBuilder::AggregateBuilder { llvm::Type *EltTy; friend class ConstantInitBuilder; + template friend class ConstantInitBuilder::AggregateBuilder; ConstantArrayBuilder(ConstantInitBuilder &builder, - AggregateBuilder *parent, llvm::Type *eltTy) + AggregateBuilderBase *parent, llvm::Type *eltTy) : AggregateBuilder(builder, parent), EltTy(eltTy) {} public: size_t size() const { @@ -206,9 +257,10 @@ public: return getBuffer().size() - Begin; } +private: /// Form an array constant from the values that have been added to this /// builder. - llvm::Constant *finish() { + llvm::Constant *finishImpl() { markFinished(); auto &buffer = getBuffer(); @@ -222,12 +274,6 @@ public: buffer.erase(buffer.begin() + Begin, buffer.end()); return constant; } - - template - llvm::GlobalVariable *finishAndCreateGlobal(As &&...args) { - assert(!Parent && "finishing non-root builder"); - return Builder.createGlobal(finish(), std::forward(args)...); - } }; inline ConstantArrayBuilder @@ -236,21 +282,23 @@ ConstantInitBuilder::beginArray(llvm::Type *eltTy) { } inline ConstantArrayBuilder -ConstantInitBuilder::AggregateBuilder::beginArray(llvm::Type *eltTy) { +ConstantInitBuilder::AggregateBuilderBase::beginArray(llvm::Type *eltTy) { return ConstantArrayBuilder(Builder, this, eltTy); } /// A helper class of ConstantInitBuilder, used for building constant /// struct initializers. -class ConstantStructBuilder : public ConstantInitBuilder::AggregateBuilder { +class ConstantStructBuilder + : public ConstantInitBuilder::AggregateBuilder { llvm::StructType *Ty; friend class ConstantInitBuilder; + template friend class ConstantInitBuilder::AggregateBuilder; ConstantStructBuilder(ConstantInitBuilder &builder, - AggregateBuilder *parent, llvm::StructType *ty) + AggregateBuilderBase *parent, llvm::StructType *ty) : AggregateBuilder(builder, parent), Ty(ty) {} -public: + /// Finish the struct. - llvm::Constant *finish(bool packed = false) { + llvm::Constant *finishImpl() { markFinished(); auto &buffer = getBuffer(); @@ -261,18 +309,12 @@ public: if (Ty) { constant = llvm::ConstantStruct::get(Ty, elts); } else { - constant = llvm::ConstantStruct::getAnon(elts, packed); + constant = llvm::ConstantStruct::getAnon(elts, /*packed*/ false); } buffer.erase(buffer.begin() + Begin, buffer.end()); return constant; } - - template - llvm::GlobalVariable *finishAndCreateGlobal(As &&...args) { - assert(!Parent && "finishing non-root builder"); - return Builder.createGlobal(finish(), std::forward(args)...); - } }; inline ConstantStructBuilder @@ -281,7 +323,8 @@ ConstantInitBuilder::beginStruct(llvm::StructType *structTy) { } inline ConstantStructBuilder -ConstantInitBuilder::AggregateBuilder::beginStruct(llvm::StructType *structTy) { +ConstantInitBuilder::AggregateBuilderBase::beginStruct( + llvm::StructType *structTy) { return ConstantStructBuilder(Builder, this, structTy); } -- GitLab