Skip to content

[ORC] Introduce RedirectionManager interface and implementation using JITLink. #66802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

sunho
Copy link
Member

@sunho sunho commented Sep 19, 2023

This patch introduces RedirectionManager and RedirectableSymbolManager interfaces which abstracts the redirection of call to certain symbols. It also adds JITLink implmenetation of such interface along with testcases covering basic use cases.

This patch is from https://reviews.llvm.org/D157859

@weliveindetail
Copy link
Member

What are the prospects for this PR? Do we want to land it in some form? I rebased it to mainline and with some minor fixes the test passes. Now that release/18.x has branched, we'd have some time to revisit it and stabilize it. What do you think?

One thing I'd like to discuss is how we can separate management of symbols from stubs. We have an IndirectStubsManager that we use for lazy JITing and it's already wired up with EPC right?

Can we integrate it in the redirection manager and avoid more stub management?

@lhames
Copy link
Contributor

lhames commented Feb 6, 2024

What are the prospects for this PR? Do we want to land it in some form? I rebased it to mainline and with some minor fixes the test passes. Now that release/18.x has branched, we'd have some time to revisit it and stabilize it. What do you think?

We definitely want to land it, I've just been short on time to review.

One thing I'd like to discuss is how we can separate management of symbols from stubs. We have an IndirectStubsManager that we use for lazy JITing and it's already wired up with EPC right?

Can we integrate it in the redirection manager and avoid more stub management?

The aim is for this work to replace the existing IndirectStubsManager (we may reuse the name, but want to update the interface).

@weliveindetail
Copy link
Member

weliveindetail commented Feb 7, 2024

The aim is for this work to replace the existing IndirectStubsManager

Ok, we might want to consider this a draft then as it seems to require more work. I think this patch is a good basis for experimentation with the design of such a new interface. Maybe it makes sense though to discuss and land the new stubs manager interface in isolation? I may find the time to think about it in the next weeks.

@sunho
Copy link
Member Author

sunho commented Apr 11, 2024

Superseded by #67050. Sorry for the confusion!

@sunho sunho closed this Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants