[llvm-commits] PATCH. Fix of hang during intel jit profiling.

[Chris Lattner]

On Aug 14, 2012, at 12:56 AM, "Uhanov, Kirill" <kirill.uhanov at intel.com> wrote:

> Hi All.
> We got approval to contribute our files under 'the University of Illinois Open Source License' .
> Could you review attached patch?

Great!  

+++ ./include/llvm/ExecutionEngine/jitprofiling.h	2012-08-13 18:25:25.000000000 +0400
@@ -0,0 +1,254 @@
+/*===-- jitprofiling.h - JIT Profiling API-------------------------*- C -*-===*

Maybe I'm not understanding the design here, but why isn't this in ExecutionEngine/IntelJITEvents?

If this needs to stay, then it is a problem that it isn't following the LLVM conventions.  If this is an imported header file, it should be sunk down into the IntelJITEvents directory and can remain relatively-unmodified.

As a random question, why is the directory named ExecutionEngine/IntelJITEvents instead of "ExecutionEngine/VTuneEvents" or something like that?  Intel has a number of JITs -- this code is an interface to VTune, not to the JITs that intel ships.

Another high level comment is that this patch includes two things: 1) importing header files, and 2) some implementation changes in IntelJITEventListener.cpp.  It would be good to separate those out to make it easier to review and understand the patch.

-Chris