Skip to content

Conversation

@kparzysz-quic
Copy link
Contributor

@kparzysz-quic kparzysz-quic requested review from Lunderberg, areusch and tqchen and removed request for Lunderberg and tqchen June 28, 2022 18:38
@tqchen
Copy link
Member

tqchen commented Jun 29, 2022

Thanks @kparzysz-quic . Reading the implementation it is unclear how cl::opt is restored atm, is it something that we need to continue work on?

@kparzysz-quic
Copy link
Contributor Author

kparzysz-quic commented Jun 29, 2022

The llvm target doesn't allow users to specify LLVM flags, so that part is not done yet. This is only about restructuring it so that there is an obvious place where the saving/restoring of options can happen. The next step would be to add "llvm-options" as an attribute to llvm, and then add code to LLVMTarget to save/restore the options' states.

Krzysztof Parzyszek added 4 commits July 13, 2022 16:17
LLVMModule has two members that refer to llvm::Module:
1. llvm::Module* mptr_, and
2. std::unique_ptr<llvm::Module> module_.

The mptr_ member is always valid and it points to the llvm::Module.
The unique pointer takes care of the ownership of the llvm::Module, and
deletes it when no longer needed. However, once JIT is initialized, it
takes over the ownership of the llvm::Module, and the module_ member
becomes null.

To avoid checks for null, use the raw pointer whenever accessing the
llvm::Module, and only use the unique_ptr as the owner/deleter.
@areusch
Copy link
Contributor

areusch commented Jul 25, 2022

@kparzysz-quic lmk when you need a review here--it's draft so i've been ignoring it

@kparzysz-quic
Copy link
Contributor Author

This was a prototype/proof-of-concept that was evolving with the RFC, so the commit history is a bit messy. I replaced this by #12140.

@kparzysz-quic kparzysz-quic deleted the llvm-target-draft branch July 25, 2022 13:26
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