[PATCH] D86435: Profiling the code generated by MCJIT engine using Intel VTune profiler

Thu Oct 29 13:41:04 PDT 2020

ekovanov added a comment.

In D86435#2360978 <https://reviews.llvm.org/D86435#2360978>, @lhames wrote:

> In D86435#2359890 <https://reviews.llvm.org/D86435#2359890>, @ekovanov wrote:
>
>> In D86435#2357748 <https://reviews.llvm.org/D86435#2357748>, @lhames wrote:
>>
>>> In D86435#2349843 <https://reviews.llvm.org/D86435#2349843>, @ekovanov wrote:
>>>
>>>> Thanks a lot for your help!
>>>> Being very new here, I do not have commit access to land this, could you please land this?
>>>
>>> Hi Elena,
>>>
>>> I missed that this included a definition of a git submodule. This would be the first in the main LLVM tree and I think we'd need to ask the community about inclusion. Is it required for this patch, or could it be dropped and downloading of that module left to clients who want to enable Intel JIT events support?
>>
>> Hi Lang!
>>
>> At my opinion, to add submodule is a good solution for open source project to be up to date with ITT API source.
>> The alternative solution - just add necessary source files from ITT API to the llvm/lib/ExecutionEngine/IntelJITEvents/ directory and manually update them from time to time.
>> How do you think, what is better to implement in my case?
>
> I'm not opposed to submodules, but I think you would need to start a thread on LLVM dev about it: I'm not the right person to approve that change.
>
> In the interim, if you would like to land the rest of this patch, do you think it would be reasonable to require clients using intel JIT events to manually create and checkout the submodule? I think that would only be a couple of extra lines when setting up an LLVM checkout, right?

Hi Lang,

I think I got your point and I fully agree with you - it does not necessary to create submodule. 
An action to clone ITT API repo (CMakeLists.txt) is enough for this case.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86435/new/

https://reviews.llvm.org/D86435