Skip to content

Remove Incorrect Extension Registration of tvm::Target - #1272

Merged
tqchen merged 1 commit into
apache:masterfrom
kazum:bsymbolic-functions-for-cmake
Jun 14, 2018
Merged

Remove Incorrect Extension Registration of tvm::Target#1272
tqchen merged 1 commit into
apache:masterfrom
kazum:bsymbolic-functions-for-cmake

Conversation

@kazum

@kazumkazum commented Jun 13, 2018

Copy link
Copy Markdown
Contributor

This PR fixes#1244 for CMake.

Here is an example configuration to produce the problem:

CMAKE_BUILD_TYPE=Debug
CMAKE_CXX_FLAGS_DEBUG="-g -O0"

@nishi-t

nishi-t commented Jun 13, 2018•
edited
Loading

Copy link
Copy Markdown
Contributor

@kazum this change causes a following error in osx:

ld: unknown option: -Bsymbolic-functions
clang: error: linker command failed with exit code 1 (use -v to see invocation)

@kazum

kazum commented Jun 13, 2018

Copy link
Copy Markdown
ContributorAuthor

@nishi-t Thanks for your comment. It looks like clang doesn't support -Bsymbolic-functions option.
I think the true fix is to use different symbols between libtvm.so and libtvm_topi.so, but it's need a quite lot of code change. For now, I'll add -Bsymbolic-functions only to GCC for a quick fix.

@kazum
kazumforce-pushed the bsymbolic-functions-for-cmake branch from 0419532 to 4c3e4a8CompareJune 13, 2018 06:03
@tqchen

tqchen commented Jun 13, 2018

Copy link
Copy Markdown
Member

Sorry I didn't take a closer look. This actually reveals a BUG that needs to be fixed. Specifically, tvm::Target is no longer an extension type, but a node type now, and we should remove the extension registeration here https://github.com/dmlc/tvm/blob/master/topi/src/topi.cc#L50

@tqchentqchen changed the title Add -Bsymbolic-functions to linker option for cmakeRemove Incorrect Extension Registration of tvm::TargetJun 13, 2018
@tqchentqchen added the status: need update need update based on feedbacks label Jun 13, 2018
@kazum

kazum commented Jun 13, 2018

Copy link
Copy Markdown
ContributorAuthor

@tqchen Okay, I'll fix the problem based on your comment, thanks.

@kazum
kazumforce-pushed the bsymbolic-functions-for-cmake branch from 4c3e4a8 to 6d11057CompareJune 14, 2018 05:11
@tqchen
tqchen merged commit 2f2f60f into apache:masterJun 14, 2018
@tqchentqchen added status: accepted and removed status: need update need update based on feedbacks labels Jun 14, 2018
@tqchen

tqchen commented Jun 14, 2018

Copy link
Copy Markdown
Member

Thanks! this is merged

tqchen pushed a commit to tqchen/tvm that referenced this pull request Jul 6, 2018
@kazum@tqchen
Remove incorrect extension registration of tvm::Target (apache#1272)
19767e7
mnuyens pushed a commit to mnuyens/tvm that referenced this pull request Jul 10, 2018
@kazum@tqchen
Remove incorrect extension registration of tvm::Target (apache#1272)
2f188c9
sergei-mironov pushed a commit to sergei-mironov/tvm that referenced this pull request Aug 8, 2018
@kazum@sergei-mironov
Remove incorrect extension registration of tvm::Target (apache#1272)
14f26a0
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tqchentqchentqchen approved these changes

Assignees

No one assigned

Labels

status: accepted

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kazum@nishi-t@tqchen