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

[Evan Cheng]

Thanks. Please commit.

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

Evan

On Jun 24, 2009, at 5:43 PM, Jeffrey Yasskin wrote:

> 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
>>
> <event-listener.patch>_______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev