[LLVMdev] [llvm-commits] JITEventListener for eventual profiling and maybe gdb support

[Jeffrey Yasskin]

On Wed, Jun 24, 2009 at 2:43 PM, Evan Cheng<evan.cheng at apple.com> wrote:
> Hi Jeffrey,
>
> This looks very good. Thanks. Some comments:
>
> +/// JitSymbolEntry - Each function that is JIT compiled results in
> one of these
> +/// being added to an array of symbols.  This indicates the name of
> the function
> +/// as well as the address range it occupies.  This allows the client
> to map
> +/// from a PC value to the name of the function.
> +struct JitSymbolEntry {
>
> A nitpick. Please rename it to "JITSymbolEntry" for naming consistency.

Done, although this is just a move.

> +
> +// This is a public symbol so the performance tools can find it.
> +JitSymbolTable *__jitSymbolTable;
> +
>
> Hmm. Is there a better solution? From what I can tell, the event
> listener is responsible for allocating symbol table memory so it
> effectively owns it. Can we register the address with the performance
> tools? We're pushing to be more thread safe.

As Eric said, this is just moving existing code, and it's #ifdef'ed
out. I'll let the Shark folks invent a better solution. :)

> +struct FunctionEmittedEvent {
> +  // Indices are local to the RecordingJITEventListener, since the
> +  // JITEventListener interface makes no guarantees about the order of
> +  // calls between Listeners.
> +  int Index;
>
> Use "unsigned" instead of "int"? Can index ever be negative?

Sure, it's unsigned now.

> On Jun 24, 2009, at 1:26 PM, Jeffrey Yasskin wrote:
>
>> Ack, sorry. I should have sent this to llvm-commits instead. :-P
>> Followups there please.
>>
>> On Wed, Jun 24, 2009 at 12:02 PM, Jeffrey
>> Yasskin<jyasskin at google.com> wrote:
>>> I intend to use this to support oprofile's ability to symbolize
>>> JITted
>>> code through the interface described at
>>> http://oprofile.sourceforge.net/doc/devel/jit-interface.html. I
>>> believe the interface will also be useful for gdb support. I'm
>>> considering adding some flags to the JITEventListener to let the JIT
>>> avoid collecting information no listener is going to use, but I won't
>>> do that until there's a need.
>>>
>>> I've added EmittedFunctionDetails in this patch so that I don't have
>>> to change the NotifyFunctionEmitted() interface in a future patch. To
>>> record line number information, oprofile wants an array of structs of
>>> the form:
>>>
>>>  struct debug_line_info {
>>>       unsigned long vma;
>>>       unsigned int lineno;
>>>       /* The filename format is unspecified, absolute path,
>>> relative etc. */
>>>       char const * filename;
>>>  };
>>>
>>> so I'll add enough information to produce that to the
>>> EmittedFunctionDetails in a later patch.
>>>
>>> Chris mentioned that someone may want to extend this to fire events
>>> on
>>> stub emission too.
>>>
>>> Let me know what you think.
>>> Jeffrey
>>>
>> <event-listener.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: event-listener.patch
Type: text/x-diff
Size: 34384 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20090624/18380581/attachment.patch>