[llvm] r339699 - [GraphDiff] Make InverseGraph a property of a GraphDiff.

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 14 14:24:08 PDT 2018


Looks like it works now. Thanks!

-- adrian

> On Aug 14, 2018, at 2:02 PM, Alina Sbirlea <asbirlea at google.com> wrote:
> 
> Hope rL339724 (D50734) fixed this (original culprit  rL339694).
> Thank you for the heads up!
> 
> 
> On Tue, Aug 14, 2018 at 11:36 AM Adrian Prantl <aprantl at apple.com> wrote:
> It looks like this (or a related) patch broke the build with -DLLVM_ENABLE_MODULES=On. Could you please take a look?
> 
> While building module 'LLVM_Backend' imported from ../lib/Target/BPF/BPF.h:14:
> While building module 'LLVM_Analysis' imported from ../include/llvm/CodeGen/MachineInstr.h:23:
> While building module 'LLVM_intrinsic_gen' imported from ../include/llvm/Analysis/MemoryLocation.h:21:
> While building module 'LLVM_IR' imported from ../include/llvm/IR/Argument.h:20:
> In file included from <module-includes>:32:
> ../include/llvm/IR/CFGDiff.h:22:10: fatal error: cyclic dependency in module 'LLVM_intrinsic_gen': LLVM_intrinsic_gen -> LLVM_IR -> LLVM_intrinsic_gen
> #include "llvm/IR/CFG.h"
>          ^
> While building module 'LLVM_Backend' imported from ../lib/Target/BPF/BPF.h:14:
> While building module 'LLVM_Analysis' imported from ../include/llvm/CodeGen/MachineInstr.h:23:
> While building module 'LLVM_intrinsic_gen' imported from ../include/llvm/Analysis/MemoryLocation.h:21:
> In file included from <module-includes>:1:
> ../include/llvm/IR/Argument.h:20:10: fatal error: could not build module 'LLVM_IR'
> #include "llvm/IR/Value.h"
>  ~~~~~~~~^~~~~~~~~~~~~~~~~
> ../include/llvm/Analysis/MemoryLocation.h:21:10: fatal error: could not build module 'LLVM_intrinsic_gen'
> #include "llvm/IR/CallSite.h"
>  ~~~~~~~~^~~~~~~~~~~~~~~~~~~~
> ../include/llvm/ProfileData/InstrProfReader.h:20:10: fatal error: could not build module 'LLVM_IR'
> #include "llvm/IR/ProfileSummary.h"
>  ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
> While building module 'LLVM_Backend' imported from ../lib/Target/BPF/BPF.h:14:
> In file included from <module-includes>:1:
> In file included from ../include/llvm/CodeGen/LatencyPriorityQueue.h:19:
> In file included from ../include/llvm/CodeGen/ScheduleDAG.h:24:
> ../include/llvm/CodeGen/MachineInstr.h:23:10: fatal error: could not build module 'LLVM_Analysis'
> #include "llvm/Analysis/AliasAnalysis.h"
>  ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from ../lib/Target/BPF/BPFAsmPrinter.cpp:15:
> ../lib/Target/BPF/BPF.h:14:10: fatal error: could not build module 'LLVM_Backend'
> #include "llvm/Target/TargetMachine.h"
>  ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 6 errors generated.
> 
> thanks!
> adrian
> 
>> On Aug 14, 2018, at 10:43 AM, Alina Sbirlea via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> 
>> Author: asbirlea
>> Date: Tue Aug 14 10:43:24 2018
>> New Revision: 339699
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=339699&view=rev
>> Log:
>> [GraphDiff] Make InverseGraph a property of a GraphDiff.
>> 
>> Summary:
>> Treating a graph in reverse is a property of the GraphDiff and should instead be a template argument, just like IsPostDom is one for DomTrees.
>> If it's just an argument to all methods, we could have mismatches between the constructor of the GraphDiff which may reverse the updates when filtering them, and the calls retrieving the filtered delete/insert updates.
>> Also, since this will be used in IDF, where we're using a DomTree, this creates a cleaner interface for the GraphTraits to use the existing template argument of DomTreeBase.
>> 
>> Separate patch from the one adding GraphDiff, so get a clear diff of what changed.
>> 
>> Reviewers: timshen, kuhar
>> 
>> Subscribers: sanjoy, llvm-commits, jlebar
>> 
>> Differential Revision: https://reviews.llvm.org/D50687
>> 
>> Modified:
>>    llvm/trunk/include/llvm/IR/CFGDiff.h
>> 
>> Modified: llvm/trunk/include/llvm/IR/CFGDiff.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/CFGDiff.h?rev=339699&r1=339698&r2=339699&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/IR/CFGDiff.h (original)
>> +++ llvm/trunk/include/llvm/IR/CFGDiff.h Tue Aug 14 10:43:24 2018
>> @@ -78,7 +78,7 @@ namespace llvm {
>> // newly inserted. The current diff treats the CFG as a graph rather than a
>> // multigraph. Added edges are pruned to be unique, and deleted edges will
>> // remove all existing edges between two blocks.
>> -template <typename NodePtr> class GraphDiff {
>> +template <typename NodePtr, bool InverseGraph = false> class GraphDiff {
>>   using UpdateMapType = SmallDenseMap<NodePtr, SmallVector<NodePtr, 2>>;
>>   UpdateMapType SuccInsert;
>>   UpdateMapType SuccDelete;
>> @@ -102,7 +102,7 @@ template <typename NodePtr> class GraphD
>> 
>> public:
>>   GraphDiff() {}
>> -  GraphDiff(ArrayRef<cfg::Update<NodePtr>> Updates, bool InverseGraph = false) {
>> +  GraphDiff(ArrayRef<cfg::Update<NodePtr>> Updates) {
>>     SmallVector<cfg::Update<NodePtr>, 4> LegalizedUpdates;
>>     cfg::LegalizeUpdates<NodePtr>(Updates, LegalizedUpdates, InverseGraph);
>>     for (auto U : LegalizedUpdates) {
>> @@ -116,8 +116,7 @@ public:
>>     }
>>   }
>> 
>> -  bool ignoreChild(const NodePtr BB, NodePtr EdgeEnd, bool InverseEdge,
>> -                   bool InverseGraph) const {
>> +  bool ignoreChild(const NodePtr BB, NodePtr EdgeEnd, bool InverseEdge) const {
>>     auto &DeleteChildren =
>>         (InverseEdge != InverseGraph) ? PredDelete : SuccDelete;
>>     auto It = DeleteChildren.find(BB);
>> @@ -128,8 +127,7 @@ public:
>>   }
>> 
>>   iterator_range<typename SmallVectorImpl<NodePtr>::const_iterator>
>> -  getAddedChildren(const NodePtr BB, bool InverseEdge,
>> -                   bool InverseGraph) const {
>> +  getAddedChildren(const NodePtr BB, bool InverseEdge) const {
>>     auto &InsertChildren =
>>         (InverseEdge != InverseGraph) ? PredInsert : SuccInsert;
>>     auto It = InsertChildren.find(BB);
>> @@ -159,7 +157,7 @@ public:
>> };
>> 
>> template <bool InverseGraph = false> struct CFGViewSuccessors {
>> -  using DataRef = const GraphDiff<BasicBlock *> *;
>> +  using DataRef = const GraphDiff<BasicBlock *, InverseGraph> *;
>>   using NodeRef = std::pair<DataRef, BasicBlock *>;
>> 
>>   using ExistingChildIterator =
>> @@ -168,7 +166,7 @@ template <bool InverseGraph = false> str
>>     BasicBlock *BB;
>>     DeletedEdgesFilter(BasicBlock *BB) : BB(BB){};
>>     bool operator()(NodeRef N) const {
>> -      return !N.first->ignoreChild(BB, N.second, false, InverseGraph);
>> +      return !N.first->ignoreChild(BB, N.second, false);
>>     }
>>   };
>>   using FilterExistingChildrenIterator =
>> @@ -182,7 +180,7 @@ template <bool InverseGraph = false> str
>>                       AddNewChildrenIterator>;
>> 
>>   static ChildIteratorType child_begin(NodeRef N) {
>> -    auto InsertVec = N.first->getAddedChildren(N.second, false, InverseGraph);
>> +    auto InsertVec = N.first->getAddedChildren(N.second, false);
>>     // filter iterator init:
>>     auto firstit = make_filter_range(
>>         make_range<ExistingChildIterator>({succ_begin(N.second), N.first},
>> @@ -197,7 +195,7 @@ template <bool InverseGraph = false> str
>>   }
>> 
>>   static ChildIteratorType child_end(NodeRef N) {
>> -    auto InsertVec = N.first->getAddedChildren(N.second, false, InverseGraph);
>> +    auto InsertVec = N.first->getAddedChildren(N.second, false);
>>     // filter iterator init:
>>     auto firstit = make_filter_range(
>>         make_range<ExistingChildIterator>({succ_end(N.second), N.first},
>> @@ -213,7 +211,7 @@ template <bool InverseGraph = false> str
>> };
>> 
>> template <bool InverseGraph = false> struct CFGViewPredecessors {
>> -  using DataRef = const GraphDiff<BasicBlock *> *;
>> +  using DataRef = const GraphDiff<BasicBlock *, InverseGraph> *;
>>   using NodeRef = std::pair<DataRef, BasicBlock *>;
>> 
>>   using ExistingChildIterator =
>> @@ -222,7 +220,7 @@ template <bool InverseGraph = false> str
>>     BasicBlock *BB;
>>     DeletedEdgesFilter(BasicBlock *BB) : BB(BB){};
>>     bool operator()(NodeRef N) const {
>> -      return !N.first->ignoreChild(BB, N.second, true, InverseGraph);
>> +      return !N.first->ignoreChild(BB, N.second, true);
>>     }
>>   };
>>   using FilterExistingChildrenIterator =
>> @@ -236,7 +234,7 @@ template <bool InverseGraph = false> str
>>                       AddNewChildrenIterator>;
>> 
>>   static ChildIteratorType child_begin(NodeRef N) {
>> -    auto InsertVec = N.first->getAddedChildren(N.second, true, InverseGraph);
>> +    auto InsertVec = N.first->getAddedChildren(N.second, true);
>>     // filter iterator init:
>>     auto firstit = make_filter_range(
>>         make_range<ExistingChildIterator>({pred_begin(N.second), N.first},
>> @@ -251,7 +249,7 @@ template <bool InverseGraph = false> str
>>   }
>> 
>>   static ChildIteratorType child_end(NodeRef N) {
>> -    auto InsertVec = N.first->getAddedChildren(N.second, true, InverseGraph);
>> +    auto InsertVec = N.first->getAddedChildren(N.second, true);
>>     // filter iterator init:
>>     auto firstit = make_filter_range(
>>         make_range<ExistingChildIterator>({pred_end(N.second), N.first},
>> @@ -267,19 +265,20 @@ template <bool InverseGraph = false> str
>> };
>> 
>> template <>
>> -struct GraphTraits<std::pair<const GraphDiff<BasicBlock *> *, BasicBlock *>>
>> +struct GraphTraits<
>> +    std::pair<const GraphDiff<BasicBlock *, false> *, BasicBlock *>>
>>     : CFGViewSuccessors<false> {};
>> template <>
>> struct GraphTraits<
>> -    std::pair<const GraphDiff<Inverse<BasicBlock *>> *, BasicBlock *>>
>> +    std::pair<const GraphDiff<BasicBlock *, true> *, BasicBlock *>>
>>     : CFGViewSuccessors<true> {};
>> template <>
>> struct GraphTraits<
>> -    std::pair<const GraphDiff<BasicBlock *> *, Inverse<BasicBlock *>>>
>> +    std::pair<const GraphDiff<BasicBlock *, false> *, Inverse<BasicBlock *>>>
>>     : CFGViewPredecessors<false> {};
>> template <>
>> struct GraphTraits<
>> -    std::pair<const GraphDiff<Inverse<BasicBlock *>> *, Inverse<BasicBlock *>>>
>> +    std::pair<const GraphDiff<BasicBlock *, true> *, Inverse<BasicBlock *>>>
>>     : CFGViewPredecessors<true> {};
>> } // end namespace llvm
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 



More information about the llvm-commits mailing list