[llvm-commits] [PATCH] JIT profiling support with Intel Parallel Amplifier XE 2011 (VTune)

Jim Grosbach grosbach at apple.com
Tue Feb 14 18:00:23 PST 2012


Hi Eli,

This all looks pretty reasonable, although I confess I'm no expert on the Intel profiling libs.

Can you elaborate on why the support is a configure time option? I assume it's due to reference Intel headers and such? Assuming so, or something along those lines, we should not build the source files for the support at all when it's not configured, rather than using #if inside the files. That is, building the code or not should be handled by the build system, not via macros in the source. Specifically, I'd really like to avoid things like "#if USE_OPROFILE" around the contents of a .cpp file.

Specific, mostly just nitty details, comments inline. I'll generally explicitly comment only comment on the first instance of any recurring patterns to avoid bloating the email more than necessary. Please do go through and look for additional instances, though. Please format to fit within 80 columns. I'm seeing lots of lines longer than that in the patch. Likewise, there's a bit of trailing whitespace that should be removed. Mostly indentation on blank lines.

Lots of this is style stuff. See http://llvm.org/docs/CodingStandards.html for the gory details.

(Chris: context sensitive question for you in the body of "OProfileWrapper")

Regards,
  Jim

ps, I still owe you a couple more emails on other JIT related stuff. Haven't forgotten!

> +
> +#ifndef INTEL_JIT_EVENTS_WRAPPER_H
> +#define INTEL_JIT_EVENTS_WRAPPER_H
> +
> +#include "jitprofiling.h"

This should use the <> style include since the file isn't in the current directory.

> +
> +namespace llvm {
> +
> +// Function pointer types for testing implementation of Intel jitprofiling
> +// library
> +typedef int (*NotifyEventPtr)(iJIT_JVM_EVENT, void*);
> +typedef void (*RegisterCallbackExPtr)(void *, iJIT_ModeChangedEx );
> +typedef iJIT_IsProfilingActiveFlags (*IsProfilingActivePtr)(void);
> +typedef void (*FinalizeThreadPtr)(void);
> +typedef void (*FinalizeProcessPtr)(void);
> +typedef unsigned int (*GetNewMethodIDPtr)(void);

These should be members of the IntelJITEventsWrapper class, not out in the general llvm namespace.

> +
> +class IntelJITEventsWrapper {
> +  NotifyEventPtr NotifyEventFunc;
> +  RegisterCallbackExPtr RegisterCallbackExFunc;
> +  IsProfilingActivePtr IsProfilingActiveFunc;
> +  FinalizeThreadPtr FinalizeThreadFunc;
> +  FinalizeProcessPtr FinalizeProcessFunc;
> +  GetNewMethodIDPtr GetNewMethodIDFunc;

Some brief comments on what these are for those not as familiar with the profiler would be awesome. Not a big deal, would just be a nice addition if it's not too much trouble.

> +
> +public:
> +  bool IsAmplifierRunning() {

Function names start with a lower case letter.

On a side note, yes LLVM is very inconsistent about this at the moment. It's a recent(ish) change to the standards and the codebase hasn't been fully migrated over yet.

> +    return IsProfilingActive() == iJIT_SAMPLING_ON;
> +  }
> +
> +  IntelJITEventsWrapper()
> +  : NotifyEventFunc(::iJIT_NotifyEvent)
> +  , RegisterCallbackExFunc(::iJIT_RegisterCallbackEx)
> +  , IsProfilingActiveFunc(::iJIT_IsProfilingActive)
> +  , FinalizeThreadFunc(::FinalizeThread)
> +  , FinalizeProcessFunc(::FinalizeProcess)
> +  , GetNewMethodIDFunc(::iJIT_GetNewMethodID) {
> +  }
> +
> +  IntelJITEventsWrapper(NotifyEventPtr notify_event_impl,
> +                   RegisterCallbackExPtr register_callback_ex_impl,
> +                   IsProfilingActivePtr is_profiling_active_impl,
> +                   FinalizeThreadPtr finalize_thread_impl,
> +                   FinalizeProcessPtr finalize_process_impl,
> +                   GetNewMethodIDPtr get_new_method_id_impl)
> +  : NotifyEventFunc(notify_event_impl)
> +  , RegisterCallbackExFunc(register_callback_ex_impl)
> +  , IsProfilingActiveFunc(is_profiling_active_impl)
> +  , FinalizeThreadFunc(finalize_thread_impl)
> +  , FinalizeProcessFunc(finalize_process_impl)
> +  , GetNewMethodIDFunc(get_new_method_id_impl) {

Commas on preceding line, not at the start of the next line.

> +  }
> +
> +  int  NotifyEvent(iJIT_JVM_EVENT EventType, void *EventSpecificData) {
> +    if (0 == NotifyEventFunc) {

LLVM style is to put the constant value on the right hand side and use the compiler warnings to catch inadvertant misuse of the assignment vs. the comparison operators.

For NULL comparisons of pointers, it's OK to just have "if (ptrval)" as well.

> +      return -1;

When there's only a single statement, in particular a single line statement, following an 'if', 'for', 'while', et. al., LLVM style is to not use a compound statement unless necessary to make control flow (nested 'if' statements, e.g.) clear.

> +    } else {

No need to have this in an 'else' clause at all since the 'true' clause doesn't fall through.

i.e., something like:
int NotifyEvent(…) {
  if (NotifyEventFunc)
    return -1;
  return NotifiyEventFunc(…);
}

Similarly for the other functions.


> +      return NotifyEventFunc(EventType, EventSpecificData);'

I gather the normal function returns more than just a success/failure code? If it's just a zero/non-zero for success vs. failure like lots of C interfaces, we should convert that to a bool for LLVM's C++ wrappers.

> +    }
> +  }
> +
> +  void RegisterCallbackEx(void *userdata, iJIT_ModeChangedEx NewModeCallBackFuncEx) {

"userdata" should be "UserData". Camelcase variable names starting w/ a capital.

> +    if (0 != RegisterCallbackExFunc) {
> +      RegisterCallbackExFunc(userdata, NewModeCallBackFuncEx);
> +    }
> +  }
> +
> +  iJIT_IsProfilingActiveFlags IsProfilingActive(void) {
> +    if (0 == IsProfilingActiveFunc) {
> +      // No jitprofiling library connection, so report nothing running
> +      return iJIT_NOTHING_RUNNING;
> +    } else {
> +      return IsProfilingActiveFunc();
> +    }
> +  }
> +
> +  void FinalizeThread(void) {
> +    if (0 != FinalizeThreadFunc) {
> +      FinalizeThreadFunc();
> +    }
> +  }
> +
> +  void FinalizeProcess(void) {
> +    if (0 != FinalizeProcessFunc) {
> +      FinalizeProcessFunc();
> +    }
> +  }
> +
> +  unsigned int GetNewMethodID(void) {
> +    if (0 == GetNewMethodIDFunc) {
> +      return -1;
> +    } else {
> +      return GetNewMethodIDFunc();
> +    }
> +  }
> +};
> +
> +} //namespace llvm
> +
> +#endif //INTEL_JIT_EVENTS_WRAPPER_H
> diff --git a/include/llvm/ExecutionEngine/JITEventListener.h b/include/llvm/ExecutionEngine/JITEventListener.h
> index 94212e1..b41ab79 100644
> --- a/include/llvm/ExecutionEngine/JITEventListener.h
> +++ b/include/llvm/ExecutionEngine/JITEventListener.h
> @@ -74,9 +74,22 @@ public:
>    virtual void NotifyFreeingMachineCode(void *) {}
>  };
>  
> -// This returns NULL if support isn't available.
> +// Construct an OProfileJITEventListener
>  JITEventListener *createOProfileJITEventListener();
>  
> +// Construct an IntelJITEventListener
> +JITEventListener *createIntelJITEventListener();
> +
> +class OProfileWrapper;

Forward class declarations should go at the top of the header file, not interspersed throughout immediately prior to usage.

> +
> +// Construct an OProfileJITEventListener with a test opagent implementation
> +JITEventListener *createOProfileJITEventListener(OProfileWrapper* alternativeImpl);
> +
> +class IntelJITEventsWrapper;
> +
> +// Construct an IntelJITEventListener with a test Intel JIT API implementation
> +JITEventListener *createIntelJITEventListener(IntelJITEventsWrapper* alternativeImpl);
> +

It strikes me that these might be better as static method on the relevant classes, if that's possible. i.e.,
JITEventListener *OProfileWrapper::Create(…);

I don't know the details of how these are used well enough to know for sure, but that would be good if it's a reasonable alternative.

>  } // end namespace llvm.
>  
>  #endif
> diff --git a/include/llvm/ExecutionEngine/OProfileWrapper.h b/include/llvm/ExecutionEngine/OProfileWrapper.h
> new file mode 100644
> index 0000000..a7533ee
> --- /dev/null
> +++ b/include/llvm/ExecutionEngine/OProfileWrapper.h
> @@ -0,0 +1,118 @@
> +//===-- OProfileWrapper.h - OProfile JIT API Wrapper -------------*- C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +// This file defines a OProfileWrapper object that detects if the oprofile
> +// daemon is running, and provides wrappers for opagent functions used to
> +// communicate with the oprofile JIT interface. The dynamic library libopagent
> +// does not need to be linked directly as this object lazily loads the library
> +// when the first op_ function is called.
> +//
> +// See http://oprofile.sourceforge.net/doc/devel/jit-interface.html for the
> +// definition of the interface.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef OPROFILE_WRAPPER_H
> +#define OPROFILE_WRAPPER_H
> +
> +#if USE_OPROFILE
> +
> +#include <stdint.h>
> +#include <opagent.h>
> +
> +namespace llvm {
> +
> +typedef  op_agent_t    (*op_open_agent_ptr_t)();
> +typedef  int           (*op_close_agent_ptr_t)(op_agent_t);
> +typedef  int           (*op_write_native_code_ptr_t)(op_agent_t, const char*, uint64_t, void const*, const unsigned int);
> +typedef  int           (*op_write_debug_line_info_ptr_t)(op_agent_t, void const*, size_t, struct debug_line_info const*);
> +typedef  int           (*op_unload_native_code_ptr_t)(op_agent_t, uint64_t);

Move into the OProfileWrapper class. Change to adhere to the LLVM naming conventions. (see below)

> +
> +// Also used for op_minor_version function which has the same signature
> +typedef  int           (*op_major_version_ptr_t)(void);
> +
> +// This is not a part of the opagent API, but is useful nonetheless
> +typedef  bool          (*IsOProfileRunningPtrT)(void);
> +
> +class OProfileWrapper {
> +  op_agent_t                      Agent;
> +  op_open_agent_ptr_t             OpenAgentFunc;
> +  op_close_agent_ptr_t            CloseAgentFunc;
> +  op_write_native_code_ptr_t      WriteNativeCodeFunc;
> +  op_write_debug_line_info_ptr_t  WriteDebugLineInfoFunc;
> +  op_unload_native_code_ptr_t     UnloadNativeCodeFunc;
> +  op_major_version_ptr_t          MajorVersionFunc;
> +  op_major_version_ptr_t          MinorVersionFunc;
> +  IsOProfileRunningPtrT           IsOProfileRunningFunc;
> +
> +  bool Initialized;
> +
> +public:
> +  OProfileWrapper();
> +
> +  //For testing with a mock opagent implementation, skips the dynamic load and the function
> +  //resolution.
> +  OProfileWrapper(op_open_agent_ptr_t OpenAgentImpl,
> +                  op_close_agent_ptr_t CloseAgentImpl,
> +                  op_write_native_code_ptr_t WriteNativeCodeImpl,
> +                  op_write_debug_line_info_ptr_t WriteDebugLineInfoImpl,
> +                  op_unload_native_code_ptr_t UnloadNativeCodeImpl,
> +                  op_major_version_ptr_t MajorVersionImpl,
> +                  op_major_version_ptr_t MinorVersionImpl,
> +                  IsOProfileRunningPtrT MockIsOProfileRunningImpl = 0)
> +  : OpenAgentFunc(OpenAgentImpl)
> +  , CloseAgentFunc(CloseAgentImpl)
> +  , WriteNativeCodeFunc(WriteNativeCodeImpl)
> +  , WriteDebugLineInfoFunc(WriteDebugLineInfoImpl)
> +  , UnloadNativeCodeFunc(UnloadNativeCodeImpl)
> +  , MajorVersionFunc(MajorVersionImpl)
> +  , MinorVersionFunc(MinorVersionImpl)
> +  , IsOProfileRunningFunc(MockIsOProfileRunningImpl)
> +  , Initialized(true)
> +  {
> +  }
> +
> +
> +  // Calls op_open_agent in the oprofile JIT library and saves the returned op_agent_t
> +  // handle internally so it can be used when calling all the other op_* functions. Callers
> +  // of this class do not need to keep track of op_agent_t objects.
> +  bool open_agent();
> +
> +  int close_agent();
> +  int write_native_code(const char* name,
> +                        uint64_t addr,
> +                        void const* code,
> +                        const unsigned int size);
> +  int write_debug_line_info(void const* code,
> +                            size_t num_entries,
> +                            struct debug_line_info const* info);
> +  int unload_native_code(uint64_t addr);
> +  int major_version(void);
> +  int minor_version(void);
> +
> +  // Returns true if the oprofiled process is running, the opagent library is loaded
> +  // and a connection to the agent has been established, and false otherwise.
> +  bool have_agent();
> +

These are wrappers for API functions of the same names in the Intel library, I gather? I can see why that would be clearer to have them be identical even though it means the names aren't following the LLVM conventions. I don't have a strong opinion either way. Chris will be better able to guide us here.


> +private:
> +  // Loads the libopagent library and initializes this wrapper if the oprofile
> +  // daemon is running
> +  bool initialize();
> +
> +  // Searches /proc for the oprofile daemon and returns true if the process if
> +  // found, or false otherwise.
> +  bool CheckForOProfileProcEntry();
> +
> +  bool IsOProfileRunning();
> +};
> +
> +} // namespace llvm
> +
> +#endif //USE_OPROFILE
> +
> +#endif //OPROFILE_WRAPPER_H
> diff --git a/lib/ExecutionEngine/JIT/EventListenerCommon.h b/lib/ExecutionEngine/JIT/EventListenerCommon.h
> new file mode 100644
> index 0000000..9ef71cd
> --- /dev/null
> +++ b/lib/ExecutionEngine/JIT/EventListenerCommon.h
> @@ -0,0 +1,63 @@
> +//===-- JIT.h - Abstract Execution Engine Interface -------------*- C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// Common functionality for JITEventListener implementations
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#ifndef EVENT_LISTENER_COMMON_H
> +#define EVENT_LISTENER_COMMON_H
> +
> +#include "llvm/Metadata.h"
> +#include "llvm/Analysis/DebugInfo.h"
> +#include "llvm/ADT/DenseMap.h"
> +#include "llvm/Support/ValueHandle.h"
> +#include "llvm/Support/Path.h"
> +
> +namespace llvm {
> +
> +class FilenameCache {

This should probably either get a less generic name since it's in a header now, or, probably better, be put in a dedicated profiler namespace.

> +  // Holds the filename of each Scope, so that we can pass a null-terminated
> +  // string into oprofile.  Use an AssertingVH rather than a ValueMap because we
> +  // shouldn't be modifying any MDNodes while this map is alive.
> +  DenseMap<AssertingVH<MDNode>, std::string> Filenames;
> +  DenseMap<AssertingVH<MDNode>, std::string> Paths;
> +
> + public:
> +  const char *getFilename(MDNode *Scope) {
> +    std::string &Filename = Filenames[Scope];
> +    if (Filename.empty()) {
> +      DIScope di_scope(Scope);
> +      Filename = di_scope.getFilename();
> +    }
> +    return Filename.c_str();
> +  }
> +
> +  const char *getFullPath(MDNode *Scope) {
> +    std::string &P = Paths[Scope];
> +    if (P.empty()) {
> +      DIScope di_scope(Scope);
> +      StringRef dir_name = di_scope.getDirectory();
> +      StringRef file_name = di_scope.getFilename();
> +      SmallString<256> full_path;
> +      if (dir_name != "." && dir_name != "") {
> +        full_path = dir_name;
> +      }
> +      if (file_name != "") {
> +        sys::path::append(full_path, file_name);
> +      }
> +      P = full_path.str();
> +    }
> +    return P.c_str();
> +  }
> +};
> +
> +} // namespace llvm
> +
> +#endif //EVENT_LISTENER_COMMON_H
> diff --git a/lib/ExecutionEngine/JIT/IntelJITEventListener.cpp b/lib/ExecutionEngine/JIT/IntelJITEventListener.cpp
> new file mode 100644
> index 0000000..b77017c
> --- /dev/null
> +++ b/lib/ExecutionEngine/JIT/IntelJITEventListener.cpp
> @@ -0,0 +1,201 @@
> +//===-- IntelJITEventListener.cpp - Tell Parallel Amplifier XE about JITted code ----===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// This file defines a JITEventListener object to tell Intel(R) VTune(TM)
> +// Amplifier XE 2011 about JITted functions.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "llvm/Config/config.h"
> +
> +#if USE_INTEL_JITEVENTS
> +
> +#define DEBUG_TYPE "amplifier-jit-event-listener"
> +#include "llvm/Function.h"
> +#include "llvm/Metadata.h"
> +#include "llvm/ADT/DenseMap.h"
> +#include "llvm/ADT/OwningPtr.h"
> +#include "llvm/Analysis/DebugInfo.h"
> +#include "llvm/CodeGen/MachineFunction.h"
> +#include "llvm/ExecutionEngine/IntelJITEventsWrapper.h"
> +#include "llvm/ExecutionEngine/JITEventListener.h"
> +#include "llvm/Support/Debug.h"
> +#include "llvm/Support/raw_ostream.h"
> +#include "llvm/Support/Errno.h"
> +#include "llvm/Support/ValueHandle.h"
> +#include "EventListenerCommon.h"
> +
> +using namespace llvm;
> +
> +namespace {
> +
> +class IntelJITEventListener : public JITEventListener {
> +  typedef DenseMap<void*, unsigned int> method_id_map_t;
> +
> +  IntelJITEventsWrapper& Wrapper;
> +  method_id_map_t MethodIDs;
> +  FilenameCache Filenames;
> +
> +public:
> +  IntelJITEventListener(IntelJITEventsWrapper& libraryWrapper)
> +  : Wrapper(libraryWrapper) {
> +  }
> +
> +  ~IntelJITEventListener() {
> +  }
> +
> +  virtual void NotifyFunctionEmitted(const Function &F,
> +                                     void *FnStart, size_t FnSize,
> +                                     const EmittedFunctionDetails &Details);
> +
> +  virtual void NotifyFreeingMachineCode(void *OldPtr);
> +};
> +
> +static LineNumberInfo LineStartToIntelJITFormat(
> +    uintptr_t StartAddress,
> +    uintptr_t Address,
> +    DebugLoc Loc) {
> +  LineNumberInfo Result;
> +
> +  Result.Offset = Address - StartAddress;
> +  Result.LineNumber = Loc.getLine();
> +
> +  return Result;
> +}
> +
> +static iJIT_Method_Load FunctionDescToIntelJITFormat(
> +    IntelJITEventsWrapper& Wrapper,
> +    const char* FnName,
> +    uintptr_t FnStart,
> +    size_t FnSize) {
> +  iJIT_Method_Load Result;
> +  memset(&Result, 0, sizeof(iJIT_Method_Load));
> +
> +  Result.method_id = Wrapper.GetNewMethodID();
> +  Result.method_name = const_cast<char*>(FnName);
> +  Result.method_load_address = reinterpret_cast<void*>(FnStart);
> +  Result.method_size = FnSize;
> +
> +  Result.class_id = 0;
> +  Result.class_file_name = NULL;
> +  Result.user_data = NULL;
> +  Result.user_data_size = 0;
> +  Result.env = iJDE_JittingAPI;
> +
> +  return Result;
> +}
> +
> +// Adds the just-emitted function to the symbol table.
> +void IntelJITEventListener::NotifyFunctionEmitted(
> +    const Function &F, void *FnStart, size_t FnSize,
> +    const EmittedFunctionDetails &Details) {
> +  iJIT_Method_Load functionMessage = FunctionDescToIntelJITFormat(Wrapper,
> +                                      F.getName().data(),
> +                                      reinterpret_cast<uint64_t>(FnStart),
> +                                      FnSize);
> +
> +  std::vector<LineNumberInfo> LineInfo;
> +
> +  if (!Details.LineStarts.empty()) {
> +    // Now convert the line number information from the address/DebugLoc
> +    // format in Details to the offset/lineno in Intel JIT API format.
> +
> +    LineInfo.reserve(Details.LineStarts.size() + 1);
> +
> +    DebugLoc FirstLoc = Details.LineStarts[0].Loc;
> +    assert(!FirstLoc.isUnknown()
> +           && "LineStarts should not contain unknown DebugLocs");
> +
> +    MDNode *FirstLocScope = FirstLoc.getScope(F.getContext());
> +    DISubprogram FunctionDI = getDISubprogram(FirstLocScope);
> +    if (FunctionDI.Verify()) {
> +      functionMessage.source_file_name = const_cast<char*>(
> +                                          Filenames.getFullPath(FirstLocScope));
> +
> +      LineNumberInfo first_line;
> +      first_line.Offset = 0;
> +      first_line.LineNumber = FunctionDI.getLineNumber();
> +      LineInfo.push_back(first_line);
> +    }
> +
> +    for (std::vector<EmittedFunctionDetails::LineStart>::const_iterator I =
> +          Details.LineStarts.begin(), E = Details.LineStarts.end();
> +          I != E; ++I) {
> +      // This implementation ignores the DebugLoc filename because the Intel
> +      // JIT API does not support multiple source files associated with a single
> +      // JIT function
> +      LineInfo.push_back(LineStartToIntelJITFormat(
> +                          reinterpret_cast<uintptr_t>(FnStart),
> +                          I->Address,
> +                          I->Loc));
> +
> +      // If we have no file name yet for the function, use the filename from
> +      // the first instruction that has one
> +      if (0 == functionMessage.source_file_name) {
> +        MDNode* scope = I->Loc.getScope(Details.MF->getFunction()->getContext());
> +        functionMessage.source_file_name = const_cast<char*>(Filenames.getFullPath(scope));
> +      }
> +    }
> +
> +    functionMessage.line_number_size = LineInfo.size();
> +    functionMessage.line_number_table = &*LineInfo.begin();
> +  } else {
> +    functionMessage.line_number_size = 0;
> +    functionMessage.line_number_table = 0;
> +  }
> +
> +  Wrapper.NotifyEvent(iJVM_EVENT_TYPE_METHOD_LOAD_FINISHED, &functionMessage);
> +  MethodIDs[FnStart] = functionMessage.method_id;
> +}
> +
> +void IntelJITEventListener::NotifyFreeingMachineCode(void *FnStart) {
> +  method_id_map_t::iterator i = MethodIDs.find(FnStart);
> +  if (i != MethodIDs.end()) {
> +    Wrapper.NotifyEvent(iJVM_EVENT_TYPE_METHOD_UNLOAD_START, &i->second);
> +    MethodIDs.erase(i);
> +  }
> +}
> +
> +}  // anonymous namespace.
> +
> +namespace llvm {
> +JITEventListener *createIntelJITEventListener() {
> +  static OwningPtr<IntelJITEventsWrapper> jitprofilingWrapper(
> +                                            new IntelJITEventsWrapper);
> +  return new IntelJITEventListener(*jitprofilingWrapper);
> +}
> +
> +// for testing
> +JITEventListener *createIntelJITEventListener(IntelJITEventsWrapper* test_impl) {
> +  return new IntelJITEventListener(*test_impl);
> +}
> +
> +} // namespace llvm
> +
> +#else  // USE_INTEL_JITEVENTS
> +
> +namespace llvm {
> +
> +class IntelJITEventsWrapper;
> +class JITEventListener;
> +
> +// By defining this to return NULL, we can let clients call it unconditionally,
> +// even if they haven't configured the build with the Intel JIT API support
> +JITEventListener *createIntelJITEventListener() {
> +  return 0;
> +}
> +
> +JITEventListener *createIntelJITEventListener(IntelJITEventsWrapper* test_impl) {
> +  return 0;
> +}
> +
> +}  // namespace llvm
> +
> +#endif  // USE_INTEL_JITEVENTS
> +
> diff --git a/lib/ExecutionEngine/JIT/OProfileJITEventListener.cpp b/lib/ExecutionEngine/JIT/OProfileJITEventListener.cpp
> index 9a9ed6d..803b475 100644
> --- a/lib/ExecutionEngine/JIT/OProfileJITEventListener.cpp
> +++ b/lib/ExecutionEngine/JIT/OProfileJITEventListener.cpp
> @@ -7,51 +7,56 @@
>  //
>  //===----------------------------------------------------------------------===//
>  //
> -// This file defines a JITEventListener object that calls into OProfile to tell
> -// it about JITted functions.  For now, we only record function names and sizes,
> -// but eventually we'll also record line number information.
> -//
> -// See http://oprofile.sourceforge.net/doc/devel/jit-interface.html for the
> -// definition of the interface we're using.
> +// This file defines a JITEventListener object that uses OProfileWrapper to tell
> +// oprofile about JITted functions, including source line information.
>  //
>  //===----------------------------------------------------------------------===//
>  
> +#include "llvm/Config/config.h"
> +
> +#if USE_OPROFILE
> +
>  #define DEBUG_TYPE "oprofile-jit-event-listener"
>  #include "llvm/Function.h"
> -#include "llvm/Metadata.h"
> -#include "llvm/ADT/DenseMap.h"
> +#include "llvm/ADT/OwningPtr.h"
>  #include "llvm/Analysis/DebugInfo.h"
>  #include "llvm/CodeGen/MachineFunction.h"
>  #include "llvm/ExecutionEngine/JITEventListener.h"
> +#include "llvm/ExecutionEngine/OProfileWrapper.h"
>  #include "llvm/Support/Debug.h"
> -#include "llvm/Support/ValueHandle.h"
>  #include "llvm/Support/raw_ostream.h"
>  #include "llvm/Support/Errno.h"
> -#include "llvm/Config/config.h"
> -#include <stddef.h>
> -using namespace llvm;
> +#include "EventListenerCommon.h"
>  
> -#if USE_OPROFILE
> +#include <dirent.h>
> +#include <fcntl.h>
>  
> -#include <opagent.h>
> +using namespace llvm;
>  
>  namespace {
>  
>  class OProfileJITEventListener : public JITEventListener {
> -  op_agent_t Agent;
> +  OProfileWrapper& Wrapper;
> +
> +  void initialize();
> +
>  public:
> -  OProfileJITEventListener();
> +  OProfileJITEventListener(OProfileWrapper& libraryWrapper)
> +  : Wrapper(libraryWrapper) {
> +    initialize();
> +  }
> +
>    ~OProfileJITEventListener();
>  
>    virtual void NotifyFunctionEmitted(const Function &F,
>                                       void *FnStart, size_t FnSize,
> -                                     const EmittedFunctionDetails &Details);
> +                                     const JITEvent_EmittedFunctionDetails &Details);
> +
>    virtual void NotifyFreeingMachineCode(void *OldPtr);
>  };
>  
> -OProfileJITEventListener::OProfileJITEventListener()
> -    : Agent(op_open_agent()) {
> -  if (Agent == NULL) {
> +void OProfileJITEventListener::initialize() {
> +  if (!Wrapper.open_agent()) {
>      const std::string err_str = sys::StrError();
>      DEBUG(dbgs() << "Failed to connect to OProfile agent: " << err_str << "\n");
>    } else {
> @@ -60,8 +65,8 @@ OProfileJITEventListener::OProfileJITEventListener()
>  }
>  
>  OProfileJITEventListener::~OProfileJITEventListener() {
> -  if (Agent != NULL) {
> -    if (op_close_agent(Agent) == -1) {
> +  if (Wrapper.have_agent()) {
> +    if (Wrapper.close_agent() == -1) {
>        const std::string err_str = sys::StrError();
>        DEBUG(dbgs() << "Failed to disconnect from OProfile agent: "
>                     << err_str << "\n");
> @@ -71,22 +76,6 @@ OProfileJITEventListener::~OProfileJITEventListener() {
>    }
>  }
>  
> -class FilenameCache {
> -  // Holds the filename of each Scope, so that we can pass a null-terminated
> -  // string into oprofile.  Use an AssertingVH rather than a ValueMap because we
> -  // shouldn't be modifying any MDNodes while this map is alive.
> -  DenseMap<AssertingVH<MDNode>, std::string> Filenames;
> -
> - public:
> -  const char *getFilename(MDNode *Scope) {
> -    std::string &Filename = Filenames[Scope];
> -    if (Filename.empty()) {
> -      Filename = DIScope(Scope).getFilename();
> -    }
> -    return Filename.c_str();
> -  }
> -};
> -
>  static debug_line_info LineStartToOProfileFormat(
>      const MachineFunction &MF, FilenameCache &Filenames,
>      uintptr_t Address, DebugLoc Loc) {
> @@ -103,9 +92,9 @@ static debug_line_info LineStartToOProfileFormat(
>  // Adds the just-emitted function to the symbol table.
>  void OProfileJITEventListener::NotifyFunctionEmitted(
>      const Function &F, void *FnStart, size_t FnSize,
> -    const EmittedFunctionDetails &Details) {
> +    const JITEvent_EmittedFunctionDetails &Details) {
>    assert(F.hasName() && FnStart != 0 && "Bad symbol to add");
> -  if (op_write_native_code(Agent, F.getName().data(),
> +  if (Wrapper.write_native_code(F.getName().data(),
>                             reinterpret_cast<uint64_t>(FnStart),
>                             FnStart, FnSize) == -1) {
>      DEBUG(dbgs() << "Failed to tell OProfile about native function "
> @@ -151,8 +140,8 @@ void OProfileJITEventListener::NotifyFunctionEmitted(
>      // line info's address to include the start of the function.
>      LineInfo[0].vma = reinterpret_cast<uintptr_t>(FnStart);
>  
> -    if (op_write_debug_line_info(Agent, FnStart,
> -                                 LineInfo.size(), &*LineInfo.begin()) == -1) {
> +    if (Wrapper.write_debug_line_info(FnStart, LineInfo.size(),
> +                                      &*LineInfo.begin()) == -1) {
>        DEBUG(dbgs()
>              << "Failed to tell OProfile about line numbers for native function "
>              << F.getName() << " at ["
> @@ -164,7 +153,7 @@ void OProfileJITEventListener::NotifyFunctionEmitted(
>  // Removes the being-deleted function from the symbol table.
>  void OProfileJITEventListener::NotifyFreeingMachineCode(void *FnStart) {
>    assert(FnStart && "Invalid function pointer");
> -  if (op_unload_native_code(Agent, reinterpret_cast<uint64_t>(FnStart)) == -1) {
> +  if (Wrapper.unload_native_code(reinterpret_cast<uint64_t>(FnStart)) == -1) {
>      DEBUG(dbgs()
>            << "Failed to tell OProfile about unload of native function at "
>            << FnStart << "\n");
> @@ -175,18 +164,38 @@ void OProfileJITEventListener::NotifyFreeingMachineCode(void *FnStart) {
>  
>  namespace llvm {
>  JITEventListener *createOProfileJITEventListener() {
> -  return new OProfileJITEventListener;
> +  static OwningPtr<OProfileWrapper> oprofileWrapper(new OProfileWrapper);
> +  return new OProfileJITEventListener(*oprofileWrapper);
>  }
> +
> +// for testing
> +JITEventListener *createOProfileJITEventListener(OProfileWrapper* test_impl) {
> +  return new OProfileJITEventListener(*test_impl);
>  }
>  
> +} // namespace llvm
> +
>  #else  // USE_OPROFILE
>  
>  namespace llvm {
> +
> +class OProfileWrapper;
> +class JITEventListener;
> +
>  // By defining this to return NULL, we can let clients call it unconditionally,
> -// even if they haven't configured with the OProfile libraries.
> -JITEventListener *createOProfileJITEventListener() {
> -  return NULL;
> +// even if they haven't configured the build with the cmake option
> +// LLVM_USE_OPROFILE (or configure flag --with-oprofile=)
> +JITEventListener *createOProfileJITEventListener()
> +{
> +  return 0;
> +}
> +
> +JITEventListener *createOProfileJITEventListener(OProfileWrapper* test_impl)
> +{
> +  return 0;
>  }
> +
>  }  // namespace llvm
>  
>  #endif  // USE_OPROFILE
> +
> diff --git a/lib/ExecutionEngine/JIT/OProfileWrapper.cpp b/lib/ExecutionEngine/JIT/OProfileWrapper.cpp
> new file mode 100644
> index 0000000..251a7dd
> --- /dev/null
> +++ b/lib/ExecutionEngine/JIT/OProfileWrapper.cpp
> @@ -0,0 +1,286 @@
> +//===-- OProfileWrapper.cpp - OProfile JIT API Wrapper implementation -----===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// This file implements the interface in OProfileWrapper.h. It is responsible
> +// for loading the opagent dynamic library when the first call to an op_
> +// function occurs.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#if USE_OPROFILE
> +
> +#include "llvm/ExecutionEngine/OProfileWrapper.h"
> +
> +#define DEBUG_TYPE "oprofile-wrapper"
> +#include "llvm/Support/Debug.h"
> +#include "llvm/Support/raw_ostream.h"
> +#include "llvm/Support/DynamicLibrary.h"
> +#include "llvm/Support/Mutex.h"
> +#include "llvm/Support/MutexGuard.h"
> +
> +#include <sstream>
> +#include <cstring>
> +#include <stddef.h>
> +#include <dirent.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +namespace {
> +
> +// Global mutex to ensure a single thread initializes oprofile agent.
> +llvm::sys::Mutex OProfileInitializationMutex;
> +
> +template<typename TO, typename FROM>
> +TO bitwise_cast(FROM source) {
> +  union {
> +    FROM src;
> +    TO dst;
> +  } u;
> +  u.src = source;
> +  return u.dst;
> +}
> +

This is just used for the function pointers, so is way overkill. Plain vanilla casting is fine. Either C or C++ style.

> +} // anonymous namespace
> +
> +namespace llvm {
> +
> +OProfileWrapper::OProfileWrapper()
> +: Agent(0)
> +, OpenAgentFunc(0)
> +, CloseAgentFunc(0)
> +, WriteNativeCodeFunc(0)
> +, WriteDebugLineInfoFunc(0)
> +, UnloadNativeCodeFunc(0)
> +, MajorVersionFunc(0)
> +, MinorVersionFunc(0)
> +, IsOProfileRunningFunc(0)
> +, Initialized(false) {
> +}
> +
> +bool OProfileWrapper::initialize() {
> +  using namespace llvm;
> +  using namespace llvm::sys;
> +
> +  llvm::MutexGuard guard(OProfileInitializationMutex);

This probably doesn't need the "llvm::" since we just had the 'using' above.

> +
> +  if (Initialized) {
> +    return OpenAgentFunc != 0;
> +  } else {
> +    Initialized = true;
> +  }
> +
> +  // If the oprofile daemon is not running, don't load the opagent library
> +  if (!IsOProfileRunning()) {
> +    DEBUG(dbgs() << "OProfile daemon is not detected.\n");
> +    return false;
> +  }
> +
> +  std::string error;
> +  if(!DynamicLibrary::LoadLibraryPermanently("libopagent.so", &error)) {

Is this support Linux only? I didn't see enforced in the configure script. If not, the name of the library is not always going to be this, most likely. If it is linux only, even just for now, the configure script should enforce that.

> +    DEBUG(dbgs() << "OProfile connector library libopagent.so could not be loaded: "
> +      << error << "\n");

Indent the "<<" to line up with the "<<" on the preceding line.

> +  }
> +
> +  // Get the addresses of the opagent functions
> +  OpenAgentFunc = bitwise_cast<op_open_agent_ptr_t>(
> +          DynamicLibrary::SearchForAddressOfSymbol("op_open_agent"));
> +  CloseAgentFunc = bitwise_cast<op_close_agent_ptr_t>(
> +          DynamicLibrary::SearchForAddressOfSymbol("op_close_agent"));
> +  WriteNativeCodeFunc = bitwise_cast<op_write_native_code_ptr_t>(
> +          DynamicLibrary::SearchForAddressOfSymbol("op_write_native_code"));
> +  WriteDebugLineInfoFunc = bitwise_cast<op_write_debug_line_info_ptr_t>(
> +          DynamicLibrary::SearchForAddressOfSymbol("op_write_debug_line_info"));
> +  UnloadNativeCodeFunc = bitwise_cast<op_unload_native_code_ptr_t>(
> +          DynamicLibrary::SearchForAddressOfSymbol("op_unload_native_code"));
> +  MajorVersionFunc = bitwise_cast<op_major_version_ptr_t>(
> +          DynamicLibrary::SearchForAddressOfSymbol("op_major_version"));
> +  MinorVersionFunc = bitwise_cast<op_major_version_ptr_t>(
> +          DynamicLibrary::SearchForAddressOfSymbol("op_minor_version"));
> +
> +  // With missing functions, we can do nothing
> +  if (!OpenAgentFunc
> +    || !CloseAgentFunc
> +    || !WriteNativeCodeFunc
> +    || !WriteDebugLineInfoFunc
> +    || !UnloadNativeCodeFunc) {

Indent the operators to line up one character after the open-paren of the 'if'.

> +    OpenAgentFunc = 0;
> +    CloseAgentFunc = 0;
> +    WriteNativeCodeFunc = 0;
> +    WriteDebugLineInfoFunc = 0;
> +    UnloadNativeCodeFunc = 0;
> +    return false;
> +  }
> +
> +  return true;
> +}
> +
> +bool OProfileWrapper::IsOProfileRunning() {
> +  if (IsOProfileRunningFunc != 0) {
> +    return IsOProfileRunningFunc();
> +  } else
> +    return CheckForOProfileProcEntry();
> +}
> +
> +bool OProfileWrapper::CheckForOProfileProcEntry() {
> +  DIR* proc_dir;
> +
> +  proc_dir = opendir("/proc");
> +  if (!proc_dir) {
> +    return false;
> +  }
> +
> +  // Walk the /proc tree looking for the oprofile daemon
> +  struct dirent* entry;
> +  while (0 != (entry = readdir(proc_dir))) {
> +    if (entry->d_type == DT_DIR) {
> +      // Build a path from the current entry name
> +      std::stringstream cmdline_fname;

LLVM uses a raw_svector_ostream writing to a SmallString for things like this.

> +      cmdline_fname << "/proc/" << entry->d_name << "/cmdline";
> +
> +      // Open the cmdline file
> +      int cmdline_fd = open(cmdline_fname.str().c_str(), S_IRUSR);
> +      if (cmdline_fd != -1) {
> +        char    exename[PATH_MAX+1];
> +        char*   basename = 0;
> +
> +        // Read the cmdline file
> +        ssize_t num_read = read(cmdline_fd, exename, PATH_MAX+1);
> +        close(cmdline_fd);
> +        ssize_t idx = 0;
> +
> +        // Find the terminator for the first string
> +        while (idx < num_read-1 && exename[idx] != 0) {
> +          idx++;
> +        }
> +
> +        // Go back to the last non-null character
> +        idx--;
> +
> +        // Find the last path separator in the first string
> +        while (idx > 0) {
> +          if (exename[idx] == '/') {
> +            basename = exename + idx + 1;
> +            break;
> +          }
> +          idx--;
> +        }
> +
> +        // Test this to see if it is the oprofile daemon
> +        if (basename != 0 && !strcmp("oprofiled", basename)) {
> +          // If it is, we're done
> +          closedir(proc_dir);
> +          return true;
> +        }
> +      }
> +    }
> +  }
> +
> +  // We've looked through all the files and didn't find the daemon
> +  closedir(proc_dir);
> +  return false;
> +}
> +
> +bool OProfileWrapper::open_agent() {
> +  if (!Initialized) {
> +    initialize();
> +  }
> +
> +  if (OpenAgentFunc != 0) {
> +    Agent = OpenAgentFunc();
> +    return Agent != 0;
> +  } else {
> +    return false;
> +  }
> +}
> +
> +int OProfileWrapper::close_agent() {
> +  if (!Initialized) {
> +    initialize();
> +  }
> +
> +  int ret = -1;
> +  if (Agent && CloseAgentFunc) {
> +    ret = CloseAgentFunc(Agent);
> +    if (0 == ret) {
> +      Agent = 0;
> +    }
> +  }
> +  return ret;
> +}
> +
> +bool OProfileWrapper::have_agent() {
> +  return Agent != 0;
> +}
> +
> +int OProfileWrapper::write_native_code(const char* name, uint64_t addr, void const* code, unsigned int size) {
> +  if (!Initialized) {
> +    initialize();
> +  }
> +
> +  if (Agent && WriteNativeCodeFunc) {
> +    return WriteNativeCodeFunc(Agent, name, addr, code, size);
> +  } else {
> +    return -1;
> +  }
> +}
> +
> +int OProfileWrapper::write_debug_line_info(
> +  void const* code,
> +  size_t num_entries,
> +  struct debug_line_info const* info) {
> +  if (!Initialized) {
> +    initialize();
> +  }
> +
> +  if (Agent && WriteDebugLineInfoFunc) {
> +    return WriteDebugLineInfoFunc(Agent, code, num_entries, info);
> +  } else {
> +    return -1;
> +  }
> +}
> +
> +int OProfileWrapper::major_version() {
> +  if (!Initialized) {
> +    initialize();
> +  }
> +
> +  if (Agent && MajorVersionFunc) {
> +    return MajorVersionFunc();
> +  } else {
> +    return -1;
> +  }
> +}
> +
> +int OProfileWrapper::minor_version() {
> +  if (!Initialized) {
> +    initialize();
> +  }
> +
> +  if (Agent && MinorVersionFunc) {
> +    return MinorVersionFunc();
> +  } else {
> +    return -1;
> +  }
> +}
> +
> +int  OProfileWrapper::unload_native_code(uint64_t addr) {
> +  if (!Initialized) {
> +    initialize();
> +  }
> +
> +  if (Agent && UnloadNativeCodeFunc) {
> +    return UnloadNativeCodeFunc(Agent, addr);
> +  } else {
> +    return -1;
> +  }
> +}
> +
> +} // namespace llvm
> +
> +#endif //USE_OPROFILE
> diff --git a/unittests/ExecutionEngine/JIT/IntelJITEventListenerTest.cpp b/unittests/ExecutionEngine/JIT/IntelJITEventListenerTest.cpp
> new file mode 100644
> index 0000000..3eed428
> --- /dev/null
> +++ b/unittests/ExecutionEngine/JIT/IntelJITEventListenerTest.cpp
> @@ -0,0 +1,111 @@
> +//===- JITEventListenerTest.cpp - Tests for Intel JIT API Events JITEventListener --------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===------------------------------------------------------------------------------------------===//
> +
> +#include "JITEventListenerTestCommon.h"
> +
> +using namespace llvm;
> +
> +#if USE_INTEL_JITEVENTS
> +
> +#include "llvm/ExecutionEngine/IntelJITEventsWrapper.h"
> +
> +#include <map>
> +#include <list>
> +
> +namespace {
> +
> +// map of function ("method") IDs to source locations
> +NativeCodeMap ReportedDebugFuncs;
> +
> +} // namespace
> +
> +/// Mock implementaion of Intel JIT API jitprofiling library
> +namespace test_jitprofiling {
> +
> +int NotifyEvent(iJIT_JVM_EVENT EventType, void *EventSpecificData) {
> +  switch (EventType) {
> +    case iJVM_EVENT_TYPE_METHOD_LOAD_FINISHED: {
> +      EXPECT_TRUE(0 != EventSpecificData);
> +      iJIT_Method_Load* msg = static_cast<iJIT_Method_Load*>(EventSpecificData);
> +      
> +      ReportedDebugFuncs[msg->method_id];
> +
> +      for(unsigned int i = 0; i < msg->line_number_size; ++i) {
> +        EXPECT_TRUE(0 != msg->line_number_table);

> +        std::pair<std::string, unsigned int> loc(std::string(msg->source_file_name),
> +          msg->line_number_table[i].LineNumber);
> +        ReportedDebugFuncs[msg->method_id].push_back(loc);
> +      }
> +    }
> +    break;
> +    case iJVM_EVENT_TYPE_METHOD_UNLOAD_START: {
> +      EXPECT_TRUE(0 != EventSpecificData);
> +      unsigned int UnloadId = *reinterpret_cast<unsigned int*>(EventSpecificData);
> +      EXPECT_TRUE(1 == ReportedDebugFuncs.erase(UnloadId));
> +    }
> +    default:
> +      break;
> +  }
> +  return 0;
> +}
> +
> +iJIT_IsProfilingActiveFlags IsProfilingActive(void) {
> +  // for testing, pretend we have an Intel Parallel Amplifier XE 2011 instance attached
> +  return iJIT_SAMPLING_ON;
> +}
> +
> +unsigned int GetNewMethodID(void) {
> +  static unsigned int id = 0;
> +  return ++id;
> +}
> +
> +} //namespace test_jitprofiling
> +
> +class IntelJITEventListenerTest
> +  : public JITEventListenerTestBase<IntelJITEventsWrapper> {
> +public:
> +  IntelJITEventListenerTest()
> +  : JITEventListenerTestBase<IntelJITEventsWrapper>(
> +      new IntelJITEventsWrapper(test_jitprofiling::NotifyEvent, 0,
> +        test_jitprofiling::IsProfilingActive, 0, 0,
> +        test_jitprofiling::GetNewMethodID))
> +  {
> +    EXPECT_TRUE(0 != MockWrapper);
> +
> +    Listener.reset(createIntelJITEventListener(MockWrapper.get()));
> +    EXPECT_TRUE(0 != Listener);
> +    EE->RegisterJITEventListener(Listener.get());
> +  }
> +};
> +
> +TEST_F(IntelJITEventListenerTest, NoDebugInfo) {
> +  TestNoDebugInfo(ReportedDebugFuncs);
> +}
> +
> +TEST_F(IntelJITEventListenerTest, SingleLine) {
> +  TestSingleLine(ReportedDebugFuncs);
> +}
> +
> +TEST_F(IntelJITEventListenerTest, MultipleLines) {
> +  TestMultipleLines(ReportedDebugFuncs);
> +}
> +  
> +
> +// This testcase is disabled because the Intel JIT API does not support a single
> +// JITted function with source lines associated with multiple files
> +/*
> +TEST_F(IntelJITEventListenerTest, MultipleFiles) {
> +  TestMultipleFiles(ReportedDebugFuncs);
> +}
> +*/
> +
> +testing::Environment* const jit_env =
> +  testing::AddGlobalTestEnvironment(new JITEnvironment);
> +
> +#endif //USE_INTEL_JITEVENTS
> diff --git a/unittests/ExecutionEngine/JIT/JITEventListenerTestCommon.h b/unittests/ExecutionEngine/JIT/JITEventListenerTestCommon.h
> new file mode 100644
> index 0000000..0067cd4
> --- /dev/null
> +++ b/unittests/ExecutionEngine/JIT/JITEventListenerTestCommon.h
> @@ -0,0 +1,194 @@
> +//===- JITEventListenerTestCommon.h- Common functionality in JITEventListener tests --------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===------------------------------------------------------------------------------------------===//
> +
> +#ifndef JIT_EVENT_LISTENER_TEST_COMMON_H
> +#define JIT_EVENT_LISTENER_TEST_COMMON_H
> +
> +#include "llvm/Analysis/DIBuilder.h"
> +#include "llvm/Analysis/DebugInfo.h"
> +#include "llvm/CodeGen/MachineCodeInfo.h"
> +#include "llvm/Config/config.h"
> +#include "llvm/ExecutionEngine/JIT.h"
> +#include "llvm/ExecutionEngine/JITEventListener.h"
> +#include "llvm/Instructions.h"
> +#include "llvm/Module.h"
> +#include "llvm/Support/IRBuilder.h"
> +#include "llvm/Support/Dwarf.h"
> +#include "llvm/Support/TypeBuilder.h"
> +#include "llvm/Support/TargetSelect.h"
> +
> +#include "gtest/gtest.h"
> +
> +#include <vector>
> +#include <string>
> +#include <utility>
> +
> +typedef std::vector<std::pair<std::string, unsigned int> > SourceLocations;
> +typedef std::map<uint64_t, SourceLocations> NativeCodeMap;
> +
> +class JITEnvironment : public testing::Environment {
> +  virtual void SetUp() {
> +    // Required to create a JIT.
> +    llvm::InitializeNativeTarget();
> +  }
> +};
> +
> +inline unsigned int getLine() {
> +  return 12;
> +}
> +
> +inline unsigned int getCol() {
> +  return 0;
> +}
> +
> +inline const char* getFilename() {
> +  return "mock_source_file.cpp";
> +}
> +
> +// Test fixture shared by tests for listener implementations
> +template<typename WrapperT>
> +class JITEventListenerTestBase : public testing::Test {
> +protected:
> +  llvm::OwningPtr<WrapperT> MockWrapper;
> +  llvm::OwningPtr<llvm::JITEventListener> Listener;
> +
> +public:
> +  llvm::Module* M;
> +  llvm::MDNode* Scope;
> +  llvm::ExecutionEngine* EE;
> +  llvm::DIBuilder* DebugBuilder;
> +  llvm::IRBuilder<> Builder;
> +
> +  JITEventListenerTestBase(WrapperT* w)
> +  : MockWrapper(w)
> +  , M(new llvm::Module("module", llvm::getGlobalContext()))
> +  , EE(llvm::EngineBuilder(M)
> +    .setEngineKind(llvm::EngineKind::JIT)
> +    .setOptLevel(llvm::CodeGenOpt::None)
> +    .create())
> +  , DebugBuilder(new llvm::DIBuilder(*M))
> +  , Builder(llvm::getGlobalContext())
> +  {
> +    DebugBuilder->createCompileUnit(llvm::dwarf::DW_LANG_C_plus_plus, "JIT", "JIT", "JIT", true, "", 1);
> +    Scope = DebugBuilder->createFile(getFilename(), ".");
> +  }
> +
> +  llvm::Function *buildFunction(const SourceLocations& DebugLocations) {
> +    using namespace llvm;
> +
> +    LLVMContext& GlobalContext = getGlobalContext();
> +
> +    SourceLocations::const_iterator CurrentDebugLocation = DebugLocations.begin();
> +
> +    if (CurrentDebugLocation != DebugLocations.end()) {
> +      DebugLoc DebugLocation = DebugLoc::get(getLine(), getCol(),
> +          DebugBuilder->createFile(CurrentDebugLocation->first, "."));
> +      Builder.SetCurrentDebugLocation(DebugLocation);
> +      CurrentDebugLocation++;
> +    }
> +
> +    Function *Result = Function::Create(
> +        TypeBuilder<int32_t(int32_t), false>::get(GlobalContext),
> +        GlobalValue::ExternalLinkage, "id", M);
> +    Value *Arg = Result->arg_begin();
> +    BasicBlock *BB = BasicBlock::Create(M->getContext(), "entry", Result);
> +    Builder.SetInsertPoint(BB);
> +    Value* one = ConstantInt::get(GlobalContext, APInt(32, 1));
> +    for(; CurrentDebugLocation != DebugLocations.end(); ++CurrentDebugLocation) {
> +      Arg = Builder.CreateMul(Arg, Builder.CreateAdd(Arg, one));
> +      Builder.SetCurrentDebugLocation(DebugLoc::get(CurrentDebugLocation->second, 0,
> +          DebugBuilder->createFile(CurrentDebugLocation->first, ".")));
> +    }
> +    Builder.CreateRet(Arg);
> +    return Result;
> +  }
> +
> +  void TestNoDebugInfo(NativeCodeMap& ReportedDebugFuncs) {
> +    SourceLocations DebugLocations;
> +    llvm::Function* f = buildFunction(DebugLocations);
> +    EXPECT_TRUE(0 != f);
> +
> +    //Cause JITting and callbacks to our listener
> +    EXPECT_TRUE(0 != EE->getPointerToFunction(f));
> +    EXPECT_TRUE(1 == ReportedDebugFuncs.size());
> +
> +    EE->freeMachineCodeForFunction(f);
> +    EXPECT_TRUE(0 == ReportedDebugFuncs.size());
> +  }
> +
> +  void TestSingleLine(NativeCodeMap& ReportedDebugFuncs) {
> +    SourceLocations DebugLocations;
> +    DebugLocations.push_back(std::make_pair(std::string(getFilename()), getLine()));
> +    llvm::Function* f = buildFunction(DebugLocations);
> +    EXPECT_TRUE(0 != f);
> +
> +    EXPECT_TRUE(0 != EE->getPointerToFunction(f));
> +    EXPECT_TRUE(1 == ReportedDebugFuncs.size());
> +    EXPECT_STREQ(ReportedDebugFuncs.begin()->second.begin()->first.c_str(), getFilename());
> +    EXPECT_EQ(ReportedDebugFuncs.begin()->second.begin()->second, getLine());
> +
> +    EE->freeMachineCodeForFunction(f);
> +    EXPECT_TRUE(0 == ReportedDebugFuncs.size());
> +  }
> +
> +  void TestMultipleLines(NativeCodeMap& ReportedDebugFuncs) {
> +    using namespace std;
> +    
> +    SourceLocations DebugLocations;
> +    unsigned int c = 5;
> +    for(unsigned int i = 0; i < c; ++i) {
> +      DebugLocations.push_back(make_pair(string(getFilename()), getLine() + i));
> +    }
> +
> +    llvm::Function* f = buildFunction(DebugLocations);
> +    EXPECT_TRUE(0 != f);
> +
> +    EXPECT_TRUE(0 != EE->getPointerToFunction(f));
> +    EXPECT_TRUE(1 == ReportedDebugFuncs.size());
> +    SourceLocations& FunctionInfo = ReportedDebugFuncs.begin()->second;
> +    EXPECT_EQ(c, FunctionInfo.size());
> +
> +    int VerifyCount = 0;
> +    for(SourceLocations::iterator i = FunctionInfo.begin(); i != FunctionInfo.end(); ++i) {
> +      EXPECT_STREQ(i->first.c_str(), getFilename());
> +      EXPECT_EQ(i->second, getLine() + VerifyCount);
> +      VerifyCount++;
> +    }
> +
> +    EE->freeMachineCodeForFunction(f);
> +    EXPECT_TRUE(0 == ReportedDebugFuncs.size());
> +  }
> +
> +  void TestMultipleFiles(NativeCodeMap& ReportedDebugFuncs) {
> +    
> +    std::string secondFilename("another_file.cpp");
> +
> +    SourceLocations DebugLocations;
> +    DebugLocations.push_back(std::make_pair(std::string(getFilename()), getLine()));
> +    DebugLocations.push_back(std::make_pair(secondFilename, getLine()));
> +    llvm::Function* f = buildFunction(DebugLocations);
> +    EXPECT_TRUE(0 != f);
> +
> +    EXPECT_TRUE(0 != EE->getPointerToFunction(f));
> +    EXPECT_TRUE(1 == ReportedDebugFuncs.size());
> +    SourceLocations& FunctionInfo = ReportedDebugFuncs.begin()->second;
> +    EXPECT_TRUE(2 == FunctionInfo.size());
> +
> +    EXPECT_STREQ(FunctionInfo.at(0).first.c_str(), getFilename());
> +    EXPECT_STREQ(FunctionInfo.at(1).first.c_str(), secondFilename.c_str());
> +
> +    EXPECT_EQ(FunctionInfo.at(0).second, getLine());
> +    EXPECT_EQ(FunctionInfo.at(1).second, getLine());
> +
> +    EE->freeMachineCodeForFunction(f);
> +    EXPECT_TRUE(0 == ReportedDebugFuncs.size());
> +  }
> +};
> +
> +#endif //JIT_EVENT_LISTENER_TEST_COMMON_H
> diff --git a/unittests/ExecutionEngine/JIT/OProfileJITEventListenerTest.cpp b/unittests/ExecutionEngine/JIT/OProfileJITEventListenerTest.cpp
> new file mode 100644
> index 0000000..de9b8d2
> --- /dev/null
> +++ b/unittests/ExecutionEngine/JIT/OProfileJITEventListenerTest.cpp
> @@ -0,0 +1,158 @@
> +//===- OProfileJITEventListenerTest.cpp - Unit tests for OProfileJITEventsListener --------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===---------------------------------------------------------------------------===//
> +
> +#if USE_OPROFILE
> +
> +#include "llvm/ExecutionEngine/OProfileWrapper.h"
> +#include "JITEventListenerTestCommon.h"
> +
> +#include <map>
> +#include <list>
> +
> +using namespace llvm;
> +
> +namespace {
> +
> +struct OprofileNativeFunction {
> +  const char* Name;
> +  uint64_t Addr;
> +  const void* CodePtr;
> +  unsigned int CodeSize;
> +
> +  OprofileNativeFunction(const char* name,
> +                         uint64_t addr,
> +                         const void* code,
> +                         unsigned int size)
> +  : Name(name)
> +  , Addr(addr)
> +  , CodePtr(code)
> +  , CodeSize(size) {
> +  }
> +};
> +
> +typedef std::list<OprofileNativeFunction> NativeFunctionList;
> +typedef std::list<debug_line_info> NativeDebugList;
> +NativeFunctionList NativeFunctions;
> +
> +NativeCodeMap ReportedDebugFuncs;
> +
> +} // namespace
> +
> +/// Mock implementaion of opagent library
> +namespace test_opagent {
> +
> +op_agent_t globalAgent = reinterpret_cast<op_agent_t>(42);
> +
> +op_agent_t open_agent()
> +{
> +  // return non-null op_agent_t
> +  return globalAgent;
> +}
> +
> +int close_agent(op_agent_t agent)
> +{
> +  EXPECT_EQ(globalAgent, agent);
> +  return 0;
> +}
> +
> +int write_native_code(op_agent_t agent, const char* name, uint64_t addr, void const* code, unsigned int size)
> +{
> +  EXPECT_EQ(globalAgent, agent);
> +  OprofileNativeFunction func(name, addr, code, size);
> +  NativeFunctions.push_back(func);
> +
> +  // Verify no other registration has take place for the same address
> +  EXPECT_TRUE(ReportedDebugFuncs.find(addr) == ReportedDebugFuncs.end());
> +
> +  ReportedDebugFuncs[addr];
> +  return 0;
> +}
> +
> +int write_debug_line_info(op_agent_t agent, void const* code, size_t num_entries, struct debug_line_info const* info)
> +{
> +  EXPECT_EQ(globalAgent, agent);
> +
> +  //verify code has been loaded first
> +  uint64_t addr = reinterpret_cast<uint64_t>(code);
> +  NativeCodeMap::iterator i = ReportedDebugFuncs.find(addr);
> +  EXPECT_TRUE(i != ReportedDebugFuncs.end());
> +
> +  NativeDebugList NativeInfo(info, info + num_entries);
> +
> +  SourceLocations locs;
> +  for(NativeDebugList::iterator i = NativeInfo.begin(); i != NativeInfo.end(); ++i) {
> +    locs.push_back(std::make_pair(std::string(i->filename), i->lineno));
> +  }
> +  ReportedDebugFuncs[addr] = locs;
> +
> +  return 0;
> +}
> +
> +int unload_native_code(op_agent_t agent, uint64_t addr) {
> +  EXPECT_EQ(globalAgent, agent);
> +
> +  //verify that something for the given JIT addr has been loaded first
> +  NativeCodeMap::iterator i = ReportedDebugFuncs.find(addr);
> +  EXPECT_TRUE(i != ReportedDebugFuncs.end());
> +  ReportedDebugFuncs.erase(i);
> +  return 0;
> +}
> +
> +int version() {
> +  return 1;
> +}
> +
> +bool is_oprofile_running() {
> +  return true;
> +}
> +
> +} //namespace test_opagent
> +
> +class OProfileJITEventListenerTest : public JITEventListenerTestBase<OProfileWrapper>
> +{
> +public:
> +  OProfileJITEventListenerTest()
> +  : JITEventListenerTestBase<OProfileWrapper>(
> +    new OProfileWrapper(test_opagent::open_agent,
> +      test_opagent::close_agent,
> +      test_opagent::write_native_code,
> +      test_opagent::write_debug_line_info,
> +      test_opagent::unload_native_code,
> +      test_opagent::version,
> +      test_opagent::version,
> +      test_opagent::is_oprofile_running))
> +  {
> +    EXPECT_TRUE(0 != MockWrapper);
> +
> +    Listener.reset(createOProfileJITEventListener(MockWrapper.get()));
> +    EXPECT_TRUE(0 != Listener);
> +    EE->RegisterJITEventListener(Listener.get());
> +  }
> +};
> +
> +TEST_F(OProfileJITEventListenerTest, NoDebugInfo) {
> +  TestNoDebugInfo(ReportedDebugFuncs);
> +}
> +
> +TEST_F(OProfileJITEventListenerTest, SingleLine) {
> +  TestSingleLine(ReportedDebugFuncs);
> +}
> +
> +TEST_F(OProfileJITEventListenerTest, MultipleLines) {
> +  TestMultipleLines(ReportedDebugFuncs);
> +}
> +
> +TEST_F(OProfileJITEventListenerTest, MultipleFiles) {
> +  TestMultipleFiles(ReportedDebugFuncs);
> +}
> +  
> +testing::Environment* const jit_env =
> +  testing::AddGlobalTestEnvironment(new JITEnvironment);
> +
> +#endif //USE_OPROFILE




On Feb 13, 2012, at 10:02 PM, Bendersky, Eli wrote:

> Ping #3 !
> 
> 
>> -----Original Message-----
>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
>> bounces at cs.uiuc.edu] On Behalf Of Bendersky, Eli
>> Sent: Wednesday, February 08, 2012 08:13
>> To: llvm-commits at cs.uiuc.edu
>> Subject: Re: [llvm-commits] [PATCH] JIT profiling support with Intel Parallel
>> Amplifier XE 2011 (VTune)
>> 
>> Ping!
>> 
>>> -----Original Message-----
>>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
>>> bounces at cs.uiuc.edu] On Behalf Of Bendersky, Eli
>>> Sent: Friday, February 03, 2012 05:18
>>> To: llvm-commits at cs.uiuc.edu
>>> Subject: Re: [llvm-commits] [PATCH] JIT profiling support with Intel
>>> Parallel Amplifier XE 2011 (VTune)
>>> 
>>> Ping!
>>> 
>>>> -----Original Message-----
>>>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
>>>> bounces at cs.uiuc.edu] On Behalf Of Bendersky, Eli
>>>> Sent: Tuesday, January 31, 2012 11:19
>>>> To: llvm-commits at cs.uiuc.edu
>>>> Subject: [llvm-commits] [PATCH] JIT profiling support with Intel
>>>> Parallel Amplifier XE 2011 (VTune)
>>>> 
>>>> Hello,
>>>> 
>>>> Currently the only profiling LLVM JITted code support is via OProfile.
>>>> This patch adds profiling support for Intel Parallel Amplifier XE
>>>> 2011 (used to be called "VTune"), does some refactoring to share
>>>> code between the implementations, and adds unit tests both for the
>>>> existing OProfile interface and the new Amplifier XE interface. In more
>> detail:
>>>> 
>>>> - Added Intel JIT Events API compatible JITEventListener, and allow
>>>> OProfileJITEventListener to load libopagent.so at runtime
>>>> - Removed link-time requirement on libopagent when building with
>>>> OProfile support
>>>> - Added Intel JIT API and OProfile support to cmake builds (Boolean
>>>> options LLVM_USE_OPROFILE and LLVM_USE_INTEL_JITEVENTS)
>>>> - Added IntelJITEventListener to connect to Intel JIT API (support
>>>> for profiling with Parallel Amplifier XE 2011)
>>>> - Added unit tests for both IntelJIT and OProfile JITEventListener
>>>> implementations which can still be run in the absence the respective
>>>> 3rd party libraries
>>>> 
>>>> The change was broken into several patches. The first contains the
>>>> new implementation and tests. The others are build system changes to
>>>> incorporate the new code.
>>>> 
>>>> This is essentially similar to the patch sent by Daniel Malea in the
>>>> beginning of December, but which unfortunately hasn't received a
>>>> reply. We updated it to cleanly apply to SVN trunk.
>>>> 
>>>> Please review
>>>> 
>>>> Eli
>>>> 
>>>> 
>>>> --------------------------------------------------------------------
>>>> -
>>>> Intel Israel (74) Limited
>>>> 
>>>> This e-mail and any attachments may contain confidential material
>>>> for the sole use of the intended recipient(s). Any review or
>>>> distribution by others is strictly prohibited. If you are not the
>>>> intended recipient, please contact the sender and delete all copies.
>>> ---------------------------------------------------------------------
>>> Intel Israel (74) Limited
>>> 
>>> This e-mail and any attachments may contain confidential material for
>>> the sole use of the intended recipient(s). Any review or distribution
>>> by others is strictly prohibited. If you are not the intended
>>> recipient, please contact the sender and delete all copies.
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> ---------------------------------------------------------------------
>> Intel Israel (74) Limited
>> 
>> This e-mail and any attachments may contain confidential material for the
>> sole use of the intended recipient(s). Any review or distribution by others is
>> strictly prohibited. If you are not the intended recipient, please contact the
>> sender and delete all copies.
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120214/f5cc9625/attachment.html>


More information about the llvm-commits mailing list