[llvm] r263275 - [ThinLTO] Support for reference graph in per-module and combined summary.
Xinliang David Li via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 21 16:12:01 PDT 2016
ok looks like a bug to fix.
thanks,
David
On Mon, Mar 21, 2016 at 4:10 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > On 2016-Mar-21, at 16:09, Xinliang David Li <xinliangli at gmail.com>
> wrote:
> >
> >
> >
> > On Mon, Mar 21, 2016 at 4:02 PM, Duncan P. N. Exon Smith via
> llvm-commits <llvm-commits at lists.llvm.org>wrote:
> >
> > > On 2016-Mar-21, at 15:13, Mehdi Amini <mehdi.amini at apple.com> wrote:
> > >
> > >>
> > >> On Mar 17, 2016, at 5:55 PM, Duncan P. N. Exon Smith via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > >>
> > >>>
> > >>> On 2016-Mar-11, at 10:52, Teresa Johnson via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> > >>>
> > >>> Author: tejohnson
> > >>> Date: Fri Mar 11 12:52:24 2016
> > >>> New Revision: 263275
> > >>>
> > >>> URL: http://llvm.org/viewvc/llvm-project?rev=263275&view=rev
> > >>> Log:
> > >>> [ThinLTO] Support for reference graph in per-module and combined
> summary.
> > >>>
> > >>> Summary:
> > >>> This patch adds support for including a full reference graph
> including
> > >>> call graph edges and other GV references in the summary.
> > >>>
> > >>> The reference graph edges can be used to make importing decisions
> > >>> without materializing any source modules, can be used in the plugin
> > >>> to make file staging decisions for distributed build systems, and is
> > >>> expected to have other uses.
> > >>>
> > >>> The call graph edges are recorded in each function summary in the
> > >>> bitcode via a list of <CalleeValueIds, StaticCount> tuples when no
> PGO
> > >>> data exists, or <CalleeValueId, StaticCount, ProfileCount> pairs when
> > >>> there is PGO, where the ValueId can be mapped to the function GUID
> via
> > >>> the ValueSymbolTable. In the function index in memory, the call graph
> > >>> edges reference the target via the CalleeGUID instead of the
> > >>> CalleeValueId.
> > >>>
> > >>> The reference graph edges are recorded in each summary record with a
> > >>> list of referenced value IDs, which can be mapped to value GUID via
> the
> > >>> ValueSymbolTable.
> > >>>
> > >>> Addtionally, a new summary record type is added to record references
> > >>> from global variable initializers. A number of bitcode records and
> data
> > >>> structures have been renamed to reflect the newly expanded scope of
> the
> > >>> summary beyond functions. More cleanup will follow.
> > >>>
> > >>> Reviewers: joker.eph, davidxl
> > >>>
> > >>> Subscribers: joker.eph, llvm-commits
> > >>>
> > >>> Differential Revision: http://reviews.llvm.org/D17212
> > >>
> > >> [snip]
> > >>
> > >>> Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
> > >>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=263275&r1=263274&r2=263275&view=diff
> > >>>
> ==============================================================================
> > >>> --- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
> > >>> +++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Fri Mar 11
> 12:52:24 2016
> > >>> @@ -2440,34 +2472,61 @@ static void WriteUseListBlock(const Func
> > >>> Stream.ExitBlock();
> > >>> }
> > >>>
> > >>> -/// \brief Save information for the given function into the
> function index.
> > >>> -///
> > >>> -/// At a minimum this saves the bitcode index of the function
> record that
> > >>> -/// was just written. However, if we are emitting function summary
> information,
> > >>> -/// for example for ThinLTO, then a \a FunctionSummary object is
> created
> > >>> -/// to hold the provided summary information.
> > >>> -static void SaveFunctionInfo(
> > >>> - const Function &F,
> > >>> - DenseMap<const Function *, std::unique_ptr<FunctionInfo>>
> &FunctionIndex,
> > >>> - unsigned NumInsts, uint64_t BitcodeIndex, bool
> EmitFunctionSummary) {
> > >>> - std::unique_ptr<FunctionSummary> FuncSummary;
> > >>> - if (EmitFunctionSummary) {
> > >>> - FuncSummary = llvm::make_unique<FunctionSummary>(NumInsts);
> > >>> - FuncSummary->setFunctionLinkage(F.getLinkage());
> > >>> +// Walk through the operands of a given User via worklist iteration
> and populate
> > >>> +// the set of GlobalValue references encountered. Invoked either on
> an
> > >>> +// Instruction or a GlobalVariable (which walks its initializer).
> > >>> +static void findRefEdges(const User *CurUser, const ValueEnumerator
> &VE,
> > >>> + DenseSet<unsigned> &RefEdges,
> > >>> + SmallPtrSet<const User *, 8> &Visited) {
> > >>> + SmallVector<const User *, 32> Worklist;
> > >>> + Worklist.push_back(CurUser);
> > >>> +
> > >>> + while (!Worklist.empty()) {
> > >>> + const User *U = Worklist.pop_back_val();
> > >>> +
> > >>> + if (!Visited.insert(U).second)
> > >>> + continue;
> > >>> +
> > >>> + ImmutableCallSite CS(U);
> > >>> +
> > >>> + for (const auto &OI : U->operands()) {
> > >>> + const User *Operand = dyn_cast<User>(OI);
> > >>> + if (!Operand)
> > >>> + continue;
> > >>> + if (isa<BlockAddress>(Operand))
> > >>> + continue;
> > >>> + if (isa<GlobalValue>(Operand)) {
> > >>> + // We have a reference to a global value. This should be
> added to
> > >>> + // the reference set unless it is a callee. Callees are
> handled
> > >>> + // specially by WriteFunction and are added to a separate
> list.
> > >>> + if (!(CS && CS.isCallee(&OI)))
> > >>> + RefEdges.insert(VE.getValueID(Operand));
> > >>> + continue;
> > >>> + }
> > >>> + Worklist.push_back(Operand);
> > >>> + }
> > >>> }
> > >>> - FunctionIndex[&F] =
> > >>> - llvm::make_unique<FunctionInfo>(BitcodeIndex,
> std::move(FuncSummary));
> > >>> }
> > >>>
> > >>> /// Emit a function body to the module stream.
> > >>> static void WriteFunction(
> > >>> - const Function &F, ValueEnumerator &VE, BitstreamWriter &Stream,
> > >>> - DenseMap<const Function *, std::unique_ptr<FunctionInfo>>
> &FunctionIndex,
> > >>> - bool EmitFunctionSummary) {
> > >>> + const Function &F, const Module *M, ValueEnumerator &VE,
> > >>> + BitstreamWriter &Stream,
> > >>> + DenseMap<const Function *, std::unique_ptr<GlobalValueInfo>>
> &FunctionIndex,
> > >>> + bool EmitSummaryIndex) {
> > >>> // Save the bitcode index of the start of this function block for
> recording
> > >>> // in the VST.
> > >>> uint64_t BitcodeIndex = Stream.GetCurrentBitNo();
> > >>>
> > >>> + bool HasProfileData = F.getEntryCount().hasValue();
> > >>> + std::unique_ptr<BlockFrequencyInfo> BFI;
> > >>> + if (EmitSummaryIndex && HasProfileData) {
> > >>> + Function &Func = const_cast<Function &>(F);
> > >>> + LoopInfo LI{DominatorTree(Func)};
> > >>> + BranchProbabilityInfo BPI{Func, LI};
> > >>> + BFI = llvm::make_unique<BlockFrequencyInfo>(Func, BPI, LI);
> > >>
> > >> This means that writing out bitcode calculates DominatorTree,
> LoopInfo,
> > >> and BlockFrequencyInfo. These are not trivial.
> > >>
> > >> I'm pretty surprised to see these here.
> > >>
> > >>> + }
> > >>> +
> > >>> Stream.EnterSubblock(bitc::FUNCTION_BLOCK_ID, 4);
> > >>> VE.incorporateFunction(F);
> > >>
> > >>
> > >>> Modified: llvm/trunk/lib/Bitcode/Writer/LLVMBuild.txt
> > >>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/LLVMBuild.txt?rev=263275&r1=263274&r2=263275&view=diff
> > >>>
> ==============================================================================
> > >>> --- llvm/trunk/lib/Bitcode/Writer/LLVMBuild.txt (original)
> > >>> +++ llvm/trunk/lib/Bitcode/Writer/LLVMBuild.txt Fri Mar 11 12:52:24
> 2016
> > >>> @@ -19,4 +19,4 @@
> > >>> type = Library
> > >>> name = BitWriter
> > >>> parent = Bitcode
> > >>> -required_libraries = Core Support
> > >>> +required_libraries = Analysis Core Support
> > >>
> > >> This means that `llvm-as` now has a dependency on lib/Analysis...
> which
> > >> is surprising.
> > >>
> > >> I feel like we've done something fundamentally wrong if we have to
> > >> run expensive analyses in order to write out IR.
> > >
> > >
> > > I agree: we should have a pass that produces the "FunctionIndex"
> itself, and the bitcode writer should be supplied with the result of
> running this pass.
> > >
> >
> > This reminds me of something Mehdi noticed earlier and mentioned
> > to me in passing, but I'm not sure anyone brought up on the list
> > yet. The current implementation of the function summary cannot
> > round-trip to textual IR. I feel like this breaks a fundamental
> > IR contract (bitcode == textual IR == IR in memory).
> >
> > Can you elaborate on this? Is it that text IR dump will drop the summary
> information Or something else? The information written out is derived from
> information from IR nothing more or less.
> >
>
> The problem is that the following doesn't round-trip:
> --
> $ cat function-summary.bc | llvm-dis | llvm-as -o function-summary2.bc
> --
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160321/70080536/attachment.html>
More information about the llvm-commits
mailing list