[llvm] r368076 - [Attributor][modulemap] Revert r368064 but fix the build

[Kristina Brooks via llvm-commits]

Author: kristina
Date: Tue Aug  6 12:53:19 2019
New Revision: 368076

URL: http://llvm.org/viewvc/llvm-project?rev=368076&view=rev
Log:
[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


Modified:
    llvm/trunk/include/llvm/Transforms/IPO/Attributor.h
    llvm/trunk/include/llvm/module.modulemap
    llvm/trunk/lib/Transforms/IPO/Attributor.cpp

Modified: llvm/trunk/include/llvm/Transforms/IPO/Attributor.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/IPO/Attributor.h?rev=368076&r1=368075&r2=368076&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/IPO/Attributor.h (original)
+++ llvm/trunk/include/llvm/Transforms/IPO/Attributor.h Tue Aug  6 12:53:19 2019
@@ -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<Attribute> &DeducedAttrs);
+};
+
 /// Helper class that provides common functionality to manifest IR attributes.
 template <Attribute::AttrKind AK, typename Base>
-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,
   ///}
 
   /// See AbstractAttribute::manifest(...).
-  virtual ChangeStatus manifest(Attributor &A);
+  ChangeStatus manifest(Attributor &A) {
+    SmallVector<Attribute, 4> 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; }

Modified: llvm/trunk/include/llvm/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/module.modulemap?rev=368076&r1=368075&r2=368076&view=diff
==============================================================================
--- llvm/trunk/include/llvm/module.modulemap (original)
+++ llvm/trunk/include/llvm/module.modulemap Tue Aug  6 12:53:19 2019
@@ -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 * }
 }
 

Modified: llvm/trunk/lib/Transforms/IPO/Attributor.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Attributor.cpp?rev=368076&r1=368075&r2=368076&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/Attributor.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/Attributor.cpp Tue Aug  6 12:53:19 2019
@@ -332,21 +332,16 @@ ChangeStatus AbstractAttribute::update(A
   return HasChanged;
 }
 
-template <Attribute::AttrKind AK, typename Base>
-ChangeStatus IRAttribute<AK, Base>::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<Attribute> &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<Attribute, 4> 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<AK, Base>::mani
     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<AK, Base>::mani
     ScopeFn.setAttributes(Attrs);
     break;
   case IRPosition::IRP_CALL_SITE_ARGUMENT:
-    CallSite(&getAnchorValue()).setAttributes(Attrs);
+    CallSite(&IRP.getAnchorValue()).setAttributes(Attrs);
   }
 
   return HasChanged;