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

[Hotfix] Mark python-FFI handling with TVM_DLL #15970

Merged
merged 2 commits into from Oct 25, 2023
Merged

Conversation

Lunderberg
Copy link
Contributor

Bugfix for builds on Windows, which has missing symbol exports.

Bugfix for builds on Windows.
@tqchen
Copy link
Member

tqchen commented Oct 24, 2023

you want to place declaration in the header like the others and put TVM_DLL there. TVM_DLL do not play well with inline extern C and need to work with

extern "C"{
TVM_DLL func
}

@junrushao
Copy link
Member

We will further need to expose those methods in a .h file, wrapped with extern "C", and mark with TVM_DLL :-( MSVC is weird

@Lunderberg Lunderberg closed this Oct 24, 2023
@Lunderberg Lunderberg deleted the ffi_ branch October 24, 2023 13:24
@Lunderberg Lunderberg restored the ffi_ branch October 24, 2023 13:26
@Lunderberg
Copy link
Contributor Author

Whoops, looks like renaming the branch on github accidentally closed the PR.

Thank you on the MSVC weirdnesses, and I've updated the PR to match the way all other C API functions are exposed. (And took it as an opportunity to add documentation on what I had previously considered internal implementation functions.)

@Lunderberg Lunderberg reopened this Oct 24, 2023
@junrushao
Copy link
Member

I'd love to get this PR in quickly as well as a hotfix, but seems i'm unable to get around the CI...

@Lunderberg
Copy link
Contributor Author

I'd love to get this PR in quickly as well as a hotfix, but seems i'm unable to get around the CI...

I think there's a skip CI flag, though whether it is [skipci], [Skip_CI], or some other value, and whether I'd need to have had it present when the PR was initially submitted, I'm not sure.

@Lunderberg Lunderberg merged commit de56d8c into apache:main Oct 25, 2023
18 of 19 checks passed
@Lunderberg Lunderberg deleted the ffi_ branch October 25, 2023 14: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.

None yet

3 participants