[llvm] r196095 - [PM] [cleanup] Rearrange the public and private sections of this class

[Chandler Carruth]

Wow, I didn't realize this was here.

This was a mistaken commit. The commit was (as the log indicates) intended
to only be the cleanup rearrangement of code.

This class doesn't work and can't possibly work as you've commented. That's
why I went on to build LazyCallGraph that actually works with the new PM.
I'll delete this since it its pretty obviously dead code. Thanks for
finding it!

On Wed, Aug 5, 2015 at 10:52 AM David Blaikie <dblaikie at gmail.com> wrote:

> Oops, was looking at the wrong ~CallGraph (one in Clang, not in LLVM).
>
> This one's a bit more involved & possibly trickier to make movable:
> http://llvm.org/docs/doxygen/html/CallGraph_8cpp_source.html#l00035
>
> Hmm, maybe not - just unique_ptrifying the relevant members and moving
> them over should probably suffice.
>
> On Wed, Aug 5, 2015 at 10:47 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Mon, Dec 2, 2013 at 4:35 AM, Chandler Carruth <chandlerc at gmail.com>
>> wrote:
>>
>>> Author: chandlerc
>>> Date: Mon Dec  2 06:35:56 2013
>>> New Revision: 196095
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=196095&view=rev
>>> Log:
>>> [PM] [cleanup] Rearrange the public and private sections of this class
>>> to be a bit more sensible. The public interface now is first followed by
>>> the implementation details.
>>>
>>> This also resolves a FIXME to make something private -- it was already
>>> possible as the one special caller was already a friend.
>>>
>>> No functionality changed.
>>>
>>> Modified:
>>>     llvm/trunk/include/llvm/Analysis/CallGraph.h
>>>
>>> Modified: llvm/trunk/include/llvm/Analysis/CallGraph.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/CallGraph.h?rev=196095&r1=196094&r2=196095&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Analysis/CallGraph.h (original)
>>> +++ llvm/trunk/include/llvm/Analysis/CallGraph.h Mon Dec  2 06:35:56 2013
>>> @@ -165,28 +165,11 @@ public:
>>>  /// Typically represents a function in the call graph. There are also
>>> special
>>>  /// "null" nodes used to represent theoretical entries in the call
>>> graph.
>>>  class CallGraphNode {
>>> -  friend class CallGraph;
>>> -
>>> -  AssertingVH<Function> F;
>>> -
>>>  public:
>>>    /// \brief A pair of the calling instruction (a call or invoke)
>>>    /// and the call graph node being called.
>>>    typedef std::pair<WeakVH, CallGraphNode *> CallRecord;
>>>
>>> -private:
>>> -  std::vector<CallRecord> CalledFunctions;
>>> -
>>> -  /// \brief The number of times that this CallGraphNode occurs in the
>>> -  /// CalledFunctions array of this or other CallGraphNodes.
>>> -  unsigned NumReferences;
>>> -
>>> -  CallGraphNode(const CallGraphNode &) LLVM_DELETED_FUNCTION;
>>> -  void operator=(const CallGraphNode &) LLVM_DELETED_FUNCTION;
>>> -
>>> -  void DropRef() { --NumReferences; }
>>> -  void AddRef() { ++NumReferences; }
>>> -
>>>  public:
>>>    typedef std::vector<CallRecord> CalledFunctionsVector;
>>>
>>> @@ -281,12 +264,48 @@ public:
>>>    /// Note that this method takes linear time, so it should be used
>>> sparingly.
>>>    void replaceCallEdge(CallSite CS, CallSite NewCS, CallGraphNode
>>> *NewNode);
>>>
>>> +private:
>>> +  friend class CallGraph;
>>> +
>>> +  AssertingVH<Function> F;
>>> +
>>> +  std::vector<CallRecord> CalledFunctions;
>>> +
>>> +  /// \brief The number of times that this CallGraphNode occurs in the
>>> +  /// CalledFunctions array of this or other CallGraphNodes.
>>> +  unsigned NumReferences;
>>> +
>>> +  CallGraphNode(const CallGraphNode &) LLVM_DELETED_FUNCTION;
>>> +  void operator=(const CallGraphNode &) LLVM_DELETED_FUNCTION;
>>> +
>>> +  void DropRef() { --NumReferences; }
>>> +  void AddRef() { ++NumReferences; }
>>> +
>>>    /// \brief A special function that should only be used by the
>>> CallGraph class.
>>> -  //
>>> -  // FIXME: Make this private?
>>>    void allReferencesDropped() { NumReferences = 0; }
>>>  };
>>>
>>> +/// \brief An analysis pass to compute the \c CallGraph for a \c Module.
>>> +///
>>> +/// This class implements the concept of an analysis pass used by the \c
>>> +/// ModuleAnalysisManager to run an analysis over a module and cache the
>>> +/// resulting data.
>>> +class CallGraphAnalysis {
>>> +public:
>>> +  /// \brief A formulaic typedef to inform clients of the result type.
>>> +  typedef CallGraph Result;
>>> +
>>> +  static void *ID() { return (void *)&PassID; }
>>> +
>>> +  /// \brief Compute the \c CallGraph for the module \c M.
>>> +  ///
>>> +  /// The real work here is done in the \c CallGraph constructor.
>>> +  CallGraph run(Module *M) { return CallGraph(*M); }
>>>
>>
>> This introduces a call to the implicit copy ctor of CallGraph which is
>> probably incorrect (since ~CallGraph involves a call to "
>> llvm::DeleteContainerSeconds(FunctionMap);").
>>
>> The type can probably be made implicitly movable by just updating
>> FunctionMap's values to be unique_ptrs, but this code also seems untested
>> (changing the definition of 'run' to be "= delete" doesn't break the
>> build). Should it be removed? updated? tested in some way?
>>
>> (found this while cleaning up -Wdeprecated warnings in LLVM)
>>
>> - David
>>
>>
>>> +
>>> +private:
>>> +  static char PassID;
>>> +};
>>> +
>>>  /// \brief The \c ModulePass which wraps up a \c CallGraph and the
>>> logic to
>>>  /// build it.
>>>  ///
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150805/84d2dc8e/attachment.html>