Skip to content
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

Handle empty LLVMModule in GetFunction #5146

Merged
merged 1 commit into from
Mar 26, 2020

Conversation

kumasento
Copy link
Contributor

#4847 introduces empty LLVMModule, which may appear when all the executable code are optimized #4748 . An empty LLVMModule is built from an IR file that only specifies target triplet and data layout.

We just notice that existing logic in LLVMModuleNode may not consider this case properly. As shown by @trevor-m in this comment, LLVMModuleNode::GetFunction doesn't work properly.

We dived into this problem and found out that LLVMModuleNode always assumes there exists an entry function (see this line). If there is not, e.g., in an empty LLVMModule, we would have trouble, mostly in the form of segfault.

This PR mainly deals with the empty LLVMModule issue in LLVMModuleNode::GetFunction. We assume that, without losing generality, a LLVMModule is empty if its entry_func_ cannot be found, i.e.:

GetGlobalAddr(runtime::symbol::tvm_module_main) == 0

We use this assumption as follows:

  1. When LazyInitJIT is called by GetFunction, we will validate the address of entry_func_ (by the condition above) first before we refer to it.
  2. If entry_func_ is empty, we can realize the current LLVMModule is empty and we should return nullptr.

Please let me know whether this help address your problem @trevor-m

@trevor-m
Copy link
Contributor

Thanks @kumasento ! I have confirmed that this fixes my issue with external codegen.

Copy link
Member

@zhiics zhiics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. LGTM. @FrozenGene please take a look.

@FrozenGene FrozenGene merged commit 08b38be into apache:master Mar 26, 2020
@FrozenGene
Copy link
Member

Thanks @kumasento for fixing it. Thanks @trevor-m @zhiics for verifying and reviewing.

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.

4 participants