Great. Thanks very much Stefan!

LGTM!

I think an orc-greedy mode for LLI sounds like a great idea. Also, once we have it we should probably make it the default mode instead of MCJIT.

Hi All,

@fhahn @xgupta Apologies for the delayed reply. I'm not sure why this was abandoned -- it looks good to me.

LGTM. Thanks Stefan!

LGTM -- Nice catch, thanks Stefan!

In D91935#2415346, @tstellar wrote:

Thanks for the explanation. Doing a git clone as part of the build is usually discouraged, though I see Lang approved the original patch. Is this code disabled by default?

Sorry -- I missed this one earlier. For what it's worth JITLink prints with wild abandon. In general I'd rather log everything and then concentrate on log formatting to make it searchable/digestible.

This has been superseded by the removable code support added in 0aec49c8531.

Hi Elena,

Committed in 1d0676b54c4.

Make executable symbols in llvm-jitlink-executor visible.

Please let me know if you have any other thoughts / comments, otherwise I'm hoping to commit this by the end of the week so we can move on to the next step: Full support for initializers in remote processes.

Address review comments.

In D90132#2370956, @sgraenitz wrote:

Ok, actually the headers don't follow the breakdown. Why is that?

Looks good to me. Thanks Moritz!

Hi Jameson, Valentin,

I'm curious to get your take on the library breakdown and corresponding headers too. We now have:

Fix diff to include original changes.

Address review comments.

Looks great to me. Thanks very much Stefan!

In D86435#2359890, @ekovanov wrote:

In D86435#2357748, @lhames wrote:

In 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?

In 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 Geoffrey,

This looks good to me.

In D86435#2348346, @clayborg wrote:

I agree that I can't see how this would affect things. Fine with this going in and taking our chances.

It's not clear to me that Elena's patch should affect this. I'm inclined to accept it and just keep an eye on the bots. If everything passes we can chalk it up to a misconfig on Harbormaster. If the bots fail we can revert, and maybe the logs will give us some more insight into what's going on.

@jingham @clayborg Do either of you recognize the error that seems to be triggered? It looks like it's consistent across the tests, or at least the ones that I looked at:

Looks good to me. Thanks Geoffrey!

In D86435#2256396, @ekovanov wrote:

Hi @lhames @andrew.w.kaylor

Looking at "Build Status" for the last diff, I can see 29 tests failed (windows) and 41 tests failed (linux).
do you think that tests failures are related to my commit?
Thank you in advance!

Yes, please go ahead and abandon this review -- it will be fixed as soon as the orcv1-removal patch lands. Thank you for the contribution though!