Hi Daniel! Daniel Dunbar wrote:> I merged in my changes to your patch, which results in the attached > patch. There may be some missed merge errors. The main problem I have > with the rest of this patch is that it causes a regression in > llvm-prof's behavior. I tried running edge profiling on the > MultiSource/Applications/aha benchmark in the llvm test-suite, and > llvm-prof no longer prints any function information: > -- > ddunbar at giles:aha$ opt -f Output/aha.linked.rbc -insert-edge-profiling -o foo.bc > ddunbar at giles:aha$ llc foo.bc -o - | gcc -x assembler - -x none > ~/llvm/Debug/lib/profile_rt.dylib > ddunbar at giles:aha$ ./a.out > ... > ddunbar at giles:aha$ ~/llvm/Release/bin/llvm-prof foo.bc llvmprof.out > [...]First of all, you have to use the original bytecode file (Output/aha.linked.bc) for use with llvm-prof like this: $ ~/llvm/Release/bin/llvm-prof Output/aha.linked.bc llvmprof.out Also you have to check that llvmprof.out is deleted between tests otherwise the figures are accumulated counts from several runs.> [...] > > The basic block counts also differ, sometimes very significantly, but > I'm not sure if this is to be expected. Can you investigate both of > these differences on some real example? I'd like to keep everything > working at least as good as it was before (modulo some other features, > like function counts or basic block traces, which we don't really > support).I have to compare the revisions 75622 and 75625 to check what's going on there, but maybe its also due to undeleted llvmprof.out files?> [...] >> Index: include/llvm/Analysis/Passes.h >> ==================================================================>> --- include/llvm/Analysis/Passes.h (revision 74697) >> +++ include/llvm/Analysis/Passes.h (working copy) > ... >> + ModulePass *createProfileLoaderPass(ProfileInfoLoader *PIL); > > I avoided the need for this by just loading the profile info twice, as > a sort of intermediate step. > >> + std::vector<unsigned> ECs = PIL->getRawEdgeCounts(); > > This copies the vector, it should be a (const) refererence. > >> + // Instrument all of the edges... >> + unsigned i = 0; >> + for (Module::iterator F = M.begin(), E = M.end(); F != E; ++F) >> + for (Function::iterator BB = F->begin(), E = F->end(); BB != E; ++BB) { >> + // Okay, we have to add a counter of each outgoing edge. If the >> + // outgoing edge is not critical don't split it, just insert the counter >> + // in the source or destination of the edge. >> + TerminatorInst *TI = BB->getTerminator(); >> + for (unsigned s = 0, e = TI->getNumSuccessors(); s != e; ++s) { >> + EdgeCounts[std::make_pair(BB, TI->getSuccessor(s))]+= ECs[i++]; > > This can end up reading off the end of ECs if the module changes (or > the profile information is of a different type). > >> Index: tools/llvm-prof/llvm-prof.cpp >> ==================================================================>> --- tools/llvm-prof/llvm-prof.cpp (revision 74697) >> +++ tools/llvm-prof/llvm-prof.cpp (working copy) > >> + class LoaderInterface : public ModulePass { >> + ProfileInfo *PI; > > I renamed this class, and moved the main printing functionality to its > run method. This avoids the member state. > >> class ProfileAnnotator : public AssemblyAnnotationWriter { >> - std::map<const Function *, unsigned> &FuncFreqs; >> - std::map<const BasicBlock*, unsigned> &BlockFreqs; >> - std::map<ProfileInfoLoader::Edge, unsigned> &EdgeFreqs; >> + ProfileInfo *PI; > > I would advocate making this a reference instead of a pointer; that > makes it obvious that this class doesn't own the ProfileInfo, and that > the client is responsible for its memory management.No objections. Thanks, Andi
Good Morning. On 22.07.09 14:47), Andreas Neustifter wrote:> Daniel Dunbar wrote: >> I merged in my changes to your patch, which results in the attached >> patch. There may be some missed merge errors. The main problem I have >> with the rest of this patch is that it causes a regression in >> llvm-prof's behavior. I tried running edge profiling on the >> MultiSource/Applications/aha benchmark in the llvm test-suite, and >> llvm-prof no longer prints any function information: >> -- >> ddunbar at giles:aha$ opt -f Output/aha.linked.rbc -insert-edge-profiling -o foo.bc >> ddunbar at giles:aha$ llc foo.bc -o - | gcc -x assembler - -x none >> ~/llvm/Debug/lib/profile_rt.dylib >> ddunbar at giles:aha$ ./a.out >> ... >> ddunbar at giles:aha$ ~/llvm/Release/bin/llvm-prof foo.bc llvmprof.out >> [...] > First of all, you have to use the original bytecode file > (Output/aha.linked.bc) for use with llvm-prof like this: > > $ ~/llvm/Release/bin/llvm-prof Output/aha.linked.bc llvmprof.out > > Also you have to check that llvmprof.out is deleted between tests > otherwise the figures are accumulated counts from several runs.What I also use to make things easier is the utils/profile.pl script, it automates the process of instrumenting running and profiling with llvm-prof. I have attached a version that produces more output and removes the llvmprof.out file in between runs automatically.>> [...] >> >> The basic block counts also differ, sometimes very significantly, but >> I'm not sure if this is to be expected. Can you investigate both of >> these differences on some real example? I'd like to keep everything >> working at least as good as it was before (modulo some other features, >> like function counts or basic block traces, which we don't really >> support). > > I have to compare the revisions 75622 and 75625 to check what's going on > there, but maybe its also due to undeleted llvmprof.out files?I have compared the revisions now and apart from the passes being executed faster (according to -time-passes) there is no difference in the llvm-prof output. Andi
Sorry, forgot attachment. On 23.07.09 07:40), Andreas Neustifter wrote:> Good Morning. > > On 22.07.09 14:47), Andreas Neustifter wrote: >> Daniel Dunbar wrote: >>> I merged in my changes to your patch, which results in the attached >>> patch. There may be some missed merge errors. The main problem I have >>> with the rest of this patch is that it causes a regression in >>> llvm-prof's behavior. I tried running edge profiling on the >>> MultiSource/Applications/aha benchmark in the llvm test-suite, and >>> llvm-prof no longer prints any function information: >>> -- >>> ddunbar at giles:aha$ opt -f Output/aha.linked.rbc -insert-edge-profiling -o foo.bc >>> ddunbar at giles:aha$ llc foo.bc -o - | gcc -x assembler - -x none >>> ~/llvm/Debug/lib/profile_rt.dylib >>> ddunbar at giles:aha$ ./a.out >>> ... >>> ddunbar at giles:aha$ ~/llvm/Release/bin/llvm-prof foo.bc llvmprof.out >>> [...] >> First of all, you have to use the original bytecode file >> (Output/aha.linked.bc) for use with llvm-prof like this: >> >> $ ~/llvm/Release/bin/llvm-prof Output/aha.linked.bc llvmprof.out >> >> Also you have to check that llvmprof.out is deleted between tests >> otherwise the figures are accumulated counts from several runs. > What I also use to make things easier is the utils/profile.pl script, it > automates the process of instrumenting running and profiling with > llvm-prof. I have attached a version that produces more output and > removes the llvmprof.out file in between runs automatically. > >>> [...] >>> >>> The basic block counts also differ, sometimes very significantly, but >>> I'm not sure if this is to be expected. Can you investigate both of >>> these differences on some real example? I'd like to keep everything >>> working at least as good as it was before (modulo some other features, >>> like function counts or basic block traces, which we don't really >>> support). >> I have to compare the revisions 75622 and 75625 to check what's going on >> there, but maybe its also due to undeleted llvmprof.out files? > I have compared the revisions now and apart from the passes being > executed faster (according to -time-passes) there is no difference in > the llvm-prof output. > > Andi-- This email is signed, for more information see http://web.student.tuwien.ac.at/~e0325716/gpg.html -------------- next part -------------- An embedded and charset-unspecified text was scrubbed... Name: profile.pl URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090723/5984e0ed/attachment.pl>
Hi Andreas, Sorry for the lag time... On Wed, Jul 22, 2009 at 5:47 AM, Andreas Neustifter<e0325716 at student.tuwien.ac.at> wrote:> Hi Daniel! > > Daniel Dunbar wrote: >> I merged in my changes to your patch, which results in the attached >> patch. There may be some missed merge errors. The main problem I have >> with the rest of this patch is that it causes a regression in >> llvm-prof's behavior. I tried running edge profiling on the >> MultiSource/Applications/aha benchmark in the llvm test-suite, and >> llvm-prof no longer prints any function information: >> -- >> ddunbar at giles:aha$ opt -f Output/aha.linked.rbc -insert-edge-profiling -o foo.bc >> ddunbar at giles:aha$ llc foo.bc -o - | gcc -x assembler - -x none >> ~/llvm/Debug/lib/profile_rt.dylib >> ddunbar at giles:aha$ ./a.out >> ... >> ddunbar at giles:aha$ ~/llvm/Release/bin/llvm-prof foo.bc llvmprof.out >> [...] > First of all, you have to use the original bytecode file > (Output/aha.linked.bc) for use with llvm-prof like this: > > $ ~/llvm/Release/bin/llvm-prof Output/aha.linked.bc llvmprof.out > > Also you have to check that llvmprof.out is deleted between tests > otherwise the figures are accumulated counts from several runs.Okay, I checked this in morning, and it seems ok. There is one quirk where the entry block to functions doesn't get a count, but I think that is cleaned up in a later patch. Can you regenerate the next patch in your sequence verse TOT and send it out again? Sorry, but I can't deal with all 8 at once -- I have to take them one at a time. :) - Daniel
Hi! Daniel Dunbar wrote:> > Sorry for the lag time...No problem. As long as I know that you get to it sometime its fine for me...> On Wed, Jul 22, 2009 at 5:47 AM, Andreas > Neustifter<e0325716 at student.tuwien.ac.at> wrote: >> Hi Daniel! >> >> Daniel Dunbar wrote: >>> I merged in my changes to your patch, which results in the attached >>> patch. There may be some missed merge errors. The main problem I have >>> with the rest of this patch is that it causes a regression in >>> llvm-prof's behavior. I tried running edge profiling on the >>> MultiSource/Applications/aha benchmark in the llvm test-suite, and >>> llvm-prof no longer prints any function information: >>> -- >>> ddunbar at giles:aha$ opt -f Output/aha.linked.rbc -insert-edge-profiling -o foo.bc >>> ddunbar at giles:aha$ llc foo.bc -o - | gcc -x assembler - -x none >>> ~/llvm/Debug/lib/profile_rt.dylib >>> ddunbar at giles:aha$ ./a.out >>> ... >>> ddunbar at giles:aha$ ~/llvm/Release/bin/llvm-prof foo.bc llvmprof.out >>> [...] >> First of all, you have to use the original bytecode file >> (Output/aha.linked.bc) for use with llvm-prof like this: >> >> $ ~/llvm/Release/bin/llvm-prof Output/aha.linked.bc llvmprof.out >> >> Also you have to check that llvmprof.out is deleted between tests >> otherwise the figures are accumulated counts from several runs. > > Okay, I checked this in morning, and it seems ok. There is one quirk > where the entry block to functions doesn't get a count, but I think > that is cleaned up in a later patch.Yes, there is tons of cleaning up in the later patches, all I know is that after the last patch I ran quite a few tests and this version runs very well.> Can you regenerate the next patch in your sequence verse TOT and send > it out again? Sorry, but I can't deal with all 8 at once -- I have to > take them one at a time. :)Of course! Here it comes, all new and shiny :-) Andi -------------- next part -------------- A non-text attachment was scrubbed... Name: llvm-r78214.added.block.function.profiling.patch Type: text/x-patch Size: 4766 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090805/2162f74a/attachment.bin>