From 26e60f065318697ed411f9b4158880d6872356ef Mon Sep 17 00:00:00 2001 From: Kristina Brooks Date: Tue, 6 Aug 2019 19:53:19 +0000 Subject: [PATCH] [Attributor][modulemap] Revert r368064 but fix the build Commit r368064 was necessary after r367953 (D65712) broke the module build. That happened, apparently, because the template class IRAttribute defined in the header had a virtual method defined in the corresponding source file (IRAttribute::manifest). To unbreak the situation this patch introduces a helper function IRAttributeManifest::manifestAttrs which is used to implement IRAttribute::manifest in the header. The deifnition of the helper function is still in the source file. Patch by jdoerfert (Johannes Doerfert) Differential Revision: https://reviews.llvm.org/D65821 llvm-svn: 368076 --- llvm/include/llvm/Transforms/IPO/Attributor.h | 16 ++++++++++++-- llvm/include/llvm/module.modulemap | 5 ----- llvm/lib/Transforms/IPO/Attributor.cpp | 21 +++++++------------ 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h index 52c9507a8bbd..1129b2884acb 100644 --- a/llvm/include/llvm/Transforms/IPO/Attributor.h +++ b/llvm/include/llvm/Transforms/IPO/Attributor.h @@ -655,9 +655,17 @@ protected: int AttributeIdx; }; +/// Helper struct necessary as the modular build fails if the virtual method +/// IRAttribute::manifest is defined in the Attributor.cpp. +struct IRAttributeManifest { + static ChangeStatus manifestAttrs(Attributor &A, IRPosition &IRP, + const ArrayRef &DeducedAttrs); +}; + /// Helper class that provides common functionality to manifest IR attributes. template -struct IRAttribute : public IRPosition, public Base { +struct IRAttribute : public IRPosition, public Base, public IRAttributeManifest { + ~IRAttribute() {} /// Constructors for the IRPosition. /// @@ -666,7 +674,11 @@ struct IRAttribute : public IRPosition, public Base { ///} /// See AbstractAttribute::manifest(...). - virtual ChangeStatus manifest(Attributor &A); + ChangeStatus manifest(Attributor &A) { + SmallVector DeducedAttrs; + getDeducedAttributes(getAnchorScope().getContext(), DeducedAttrs); + return IRAttributeManifest::manifestAttrs(A, getIRPosition(), DeducedAttrs); + } /// Return the kind that identifies the abstract attribute implementation. Attribute::AttrKind getAttrKind() const { return AK; } diff --git a/llvm/include/llvm/module.modulemap b/llvm/include/llvm/module.modulemap index abfbd0b68103..ecb3b37004fd 100644 --- a/llvm/include/llvm/module.modulemap +++ b/llvm/include/llvm/module.modulemap @@ -333,11 +333,6 @@ module LLVM_Transforms { requires cplusplus umbrella "Transforms" - // FIXME: This is far from a perfect solution but at the moment this header - // is difficult to modularize and would require splitting it up. Exclude it - // completely to avoid a link failure with modular builds. - exclude header "Transforms/IPO/Attributor.h" - module * { export * } } diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp index 22c6d4fa7f6a..d98028a64453 100644 --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -332,21 +332,16 @@ ChangeStatus AbstractAttribute::update(Attributor &A, return HasChanged; } -template -ChangeStatus IRAttribute::manifest(Attributor &A) { - assert(this->getState().isValidState() && - "Attempted to manifest an invalid state!"); - assert(getIRPosition().getAssociatedValue() && + ChangeStatus IRAttributeManifest::manifestAttrs(Attributor &A, IRPosition + &IRP, const ArrayRef &DeducedAttrs) { + assert(IRP.getAssociatedValue() && "Attempted to manifest an attribute without associated value!"); ChangeStatus HasChanged = ChangeStatus::UNCHANGED; - Function &ScopeFn = getAnchorScope(); + Function &ScopeFn = IRP.getAnchorScope(); LLVMContext &Ctx = ScopeFn.getContext(); - IRPosition::Kind PK = getPositionKind(); - - SmallVector DeducedAttrs; - getDeducedAttributes(Ctx, DeducedAttrs); + IRPosition::Kind PK = IRP.getPositionKind(); // In the following some generic code that will manifest attributes in // DeducedAttrs if they improve the current IR. Due to the different @@ -360,12 +355,12 @@ ChangeStatus IRAttribute::manifest(Attributor &A) { Attrs = ScopeFn.getAttributes(); break; case IRPosition::IRP_CALL_SITE_ARGUMENT: - Attrs = ImmutableCallSite(&getAnchorValue()).getAttributes(); + Attrs = ImmutableCallSite(&IRP.getAnchorValue()).getAttributes(); break; } for (const Attribute &Attr : DeducedAttrs) { - if (!addIfNotExistent(Ctx, Attr, Attrs, getAttrIdx())) + if (!addIfNotExistent(Ctx, Attr, Attrs, IRP.getAttrIdx())) continue; HasChanged = ChangeStatus::CHANGED; @@ -382,7 +377,7 @@ ChangeStatus IRAttribute::manifest(Attributor &A) { ScopeFn.setAttributes(Attrs); break; case IRPosition::IRP_CALL_SITE_ARGUMENT: - CallSite(&getAnchorValue()).setAttributes(Attrs); + CallSite(&IRP.getAnchorValue()).setAttributes(Attrs); } return HasChanged;