JITEventListener for eventual profiling and maybe gdb support

I intend to use this to support oprofile's ability to symbolize JITted
code through the interface described at
2. Implementing JIT support for a new virtual machine. 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 (33.5 KB)

Ack, sorry. I should have sent this to llvm-commits instead. :stuck_out_tongue:
Followups there please.

event-listener.patch (33.5 KB)

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.

This code is just a move of something already in llvm. I'm currently bugging
the performance tools guys here for a better way to do this, and have hopes
of something reasonably soon I can add into llvm. That said, as far as I can tell
from asking this code doesn't work with any performance tools at apple
and is off by default :slight_smile:

-eric

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. :slight_smile:

+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.

event-listener.patch (33.6 KB)

Thanks. Please commit.

Owen, just a FYI about __jitSymbolTable. I thought you should care because of your thread safe work.

Evan