From: Maynard J. <may...@us...> - 2008-02-05 21:52:54
|
Bob Nelson wrote: > When a program running on a Cell SPU needs to perform an operation such > as I/O the request is sent to the PPU. In order to do this the runtime > functions generally call __send_to_ppe which creates a small code stub > on the stack and then jumps to the stub. Since the stub's address does > not show up in the program's memory map OProfile would normally ignore > this address. This change creates an artificial symbol > (__send_to_ppe(stack)) for Cell SPU profiles. Addresses captured that > are not in the program's memory map are attributed to this symbol. > > There was also another case in the code in op_bfd.cpp that an artificial > symbol was created with a FIXME note by it. I changed that part of the > code to use the new boolean I added to op_bfd_symbol to be consistent. Looks fine to me and works OK, too. **John**, just a little more detail about the need for this patch . . . The call stubs for __send_to_ppe are put on the SPU stack when a Cell SPE app has a need to make a (expensive) call to a "standard" library function -- i.e., from a library that's loaded in main memory and accessible to the PPU. So the mbox communication protocol is used to execute this "function call". This is a synchronous call, so the SPU program counter sits on this call stub, waiting for results to return. Since this call stub is put on the SPU's stack, opreport can't correlate the sampled address to a valid address within the SPU app, so the time spent here never appears in the report. If you approve the patch, I can commit it. Thanks. -Maynard > > Bob Nelson > > Index: oprofile-0.9.3-cvs/libutil++/op_bfd.h > =================================================================== > --- oprofile-0.9.3-cvs.orig/libutil++/op_bfd.h > +++ oprofile-0.9.3-cvs/libutil++/op_bfd.h > @@ -44,6 +44,7 @@ public: > > /// ctor for artificial symbols > op_bfd_symbol(bfd_vma vma, size_t size, std::string const & name); > + op_bfd_symbol(bfd_vma vma, unsigned long fp, size_t size, std::string const & name); > > bfd_vma vma() const { return symb_value + section_vma; } > unsigned long value() const { return symb_value; } > @@ -54,6 +55,7 @@ public: > void size(size_t s) { symb_size = s; } > bool hidden() const { return symb_hidden; } > bool weak() const { return symb_weak; } > + bool artificial() const { return symb_artificial; } > > /// compare two symbols by their filepos() > bool operator<(op_bfd_symbol const & lhs) const; > @@ -77,6 +79,8 @@ private: > bool symb_hidden; > /// whether other symbols can override it > bool symb_weak; > + /// symbol is artificially created > + bool symb_artificial; > /// code bytes corresponding to symbol -- used for XML generation > std::string symb_bytes; > }; > Index: oprofile-0.9.3-cvs/libutil++/op_bfd.cpp > =================================================================== > --- oprofile-0.9.3-cvs.orig/libutil++/op_bfd.cpp > +++ oprofile-0.9.3-cvs/libutil++/op_bfd.cpp > @@ -57,7 +57,8 @@ op_bfd_symbol::op_bfd_symbol(asymbol con > : bfd_symbol(a), symb_value(a->value), > section_filepos(a->section->filepos), > section_vma(a->section->vma), > - symb_size(0), symb_hidden(false), symb_weak(false) > + symb_size(0), symb_hidden(false), symb_weak(false), > + symb_artificial(false) > { > // Some sections have unnamed symbols in them. If > // we just ignore them then we end up sticking > @@ -77,7 +78,17 @@ op_bfd_symbol::op_bfd_symbol(asymbol con > op_bfd_symbol::op_bfd_symbol(bfd_vma vma, size_t size, string const & name) > : bfd_symbol(0), symb_value(vma), > section_filepos(0), section_vma(0), > - symb_size(size), symb_name(name) > + symb_size(size), symb_name(name), > + symb_artificial(true) > +{ > +} > + > + > +op_bfd_symbol::op_bfd_symbol(bfd_vma vma, unsigned long fp, size_t size, string const & name) > + : bfd_symbol(0), symb_value(0), > + section_filepos(fp), section_vma(vma), > + symb_size(size), symb_name(name), > + symb_artificial(true) > { > } > > @@ -296,7 +307,7 @@ get_symbol_contents(symbol_index_t sym_i > op_bfd_symbol const & bfd_sym = syms[sym_index]; > size_t size = bfd_sym.size(); > string const name = bfd_sym.name(); > - if (name.size() == 0 || name[0] == '?' || !ibfd.valid()) > + if (name.size() == 0 || bfd_sym.artificial() || !ibfd.valid()) > return false; > > if (!bfd_get_section_contents(ibfd.abfd, bfd_sym.symbol()->section, > @@ -413,14 +424,10 @@ void op_bfd::get_vma_range(bfd_vma & sta > > op_bfd_symbol const op_bfd::create_artificial_symbol() > { > - // FIXME: prefer a bool artificial; to this ?? > - string symname = "?"; > - > - symname += get_filename(); > > bfd_vma start, end; > get_vma_range(start, end); > - return op_bfd_symbol(start, end - start, symname); > + return op_bfd_symbol(start, end - start, get_filename()); > } > > > Index: oprofile-0.9.3-cvs/libutil++/op_spu_bfd.cpp > =================================================================== > --- oprofile-0.9.3-cvs.orig/libutil++/op_spu_bfd.cpp > +++ oprofile-0.9.3-cvs/libutil++/op_spu_bfd.cpp > @@ -16,6 +16,7 @@ > #include <iostream> > > #include "op_bfd.h" > +#include "op_spu_bfd.h" > #include "locate_images.h" > #include "op_libiberty.h" > #include "string_filter.h" > @@ -158,6 +159,11 @@ find_sec_code: > > get_symbols(symbols); > > + /* In some cases the SPU library code generates code stubs on the stack. */ > + /* The kernel module remaps those addresses so add an entry to catch/report them. */ > + symbols.push_back(op_bfd_symbol(OP_SPU_DYN_FLAG, OP_SPU_DYN_FLAG, > + OP_SPU_MEMSIZE, "__send_to_ppe(stack)")); > + > out: > add_symbols(symbols, symbol_filter); > return; > Index: oprofile-0.9.3-cvs/libutil++/op_spu_bfd.h > =================================================================== > --- /dev/null > +++ oprofile-0.9.3-cvs/libutil++/op_spu_bfd.h > @@ -0,0 +1,18 @@ > +/** > + * @file op_spu_bfd.h > + * > + * @remark Copyright 2007 OProfile authors > + * @remark Read the file COPYING > + * > + * @author Bob Nelson > + * (C) Copyright IBM Corporation 2007 > + */ > + > +#ifndef OP_SPU_BFD_H > +#define OP_SPU_BFD_H > + > +#define OP_SPU_DYN_FLAG 0x10000000 /* kernel module adds this offset */ > + /* to SPU code it can't find in the map */ > +#define OP_SPU_MEMSIZE 0x3ffff /* Physical memory size on an SPU */ > + > +#endif /* !OP_SPU_BFD_H */ > Index: oprofile-0.9.3-cvs/ChangeLog > =================================================================== > --- oprofile-0.9.3-cvs.orig/ChangeLog > +++ oprofile-0.9.3-cvs/ChangeLog > @@ -1,5 +1,12 @@ > 2008-02-05 Bob Nelson <rrn...@us...> > > + * libutil++/op_bfd.h: add bool symb_artificial to op_bfd_symbol > + * libutil++/op_bfd.cpp: ctor changes/add, use symb_artificial > + * libutil++/op_spu_bfd.cpp: profile SPU 'stack' code > + * libutil++/op_spu_bfd.h: new file > + > +2008-02-05 Bob Nelson <rrn...@us...> > + > * utils/opcontrol: Fix loop in dump code when using > --session-dir on a network drive. (clock issues) > |