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

Jump to bottom

[RUST][FRONTEND] Add rust frontend v0.1 #2292

Merged
merged 20 commits into from Feb 3, 2019
Merged

[RUST][FRONTEND] Add rust frontend v0.1 #2292

merged 20 commits into from Feb 3, 2019

Conversation

ehsanmok
Copy link
Contributor

@ehsanmok ehsanmok commented Dec 14, 2018 •
edited

Overview

I'm delighted to submit this PR which adds Rust runtime frontend support as part of RFC 1601.

You can see the dev here. Here's a summary

  1. Add tvm_frontend bindings and idiomatic Rust API.
  2. Create a common crate refactoring TVMArgValue and TVMRetValue and unifying FFI across runtime and frontend.
  3. Various cleaning up and adding more documentations.

@nhynes @tqchen @jroesch

resolves #2306

@ehsanmok ehsanmok mentioned this pull request Dec 14, 2018
Closed
@nhynes nhynes self-requested a review December 14, 2018 18:04
nhynes
nhynes suggested changes Dec 14, 2018
View reviewed changes
rust/Cargo.toml Outdated Show resolved Hide resolved
rust/frontend/.gitignore Show resolved Hide resolved
rust/frontend/.rustfmt.toml Outdated Show resolved Hide resolved
rust/frontend/.travis.yml Outdated Show resolved Hide resolved
rust/frontend/Cargo.toml Outdated Show resolved Hide resolved
rust/frontend/src/value.rs Outdated Show resolved Hide resolved
rust/frontend/tvm-sys/Cargo.toml Outdated Show resolved Hide resolved
rust/frontend/tvm-sys/src/bindgen.rs Outdated Show resolved Hide resolved
rust/frontend/tvm-sys/build.rs Outdated Show resolved Hide resolved
rust/runtime/.gitignore Outdated Show resolved Hide resolved
nhynes
nhynes suggested changes Dec 16, 2018
View reviewed changes
Copy link
Member

@nhynes nhynes left a comment

Choose a reason for hiding this comment

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

nice work so far! a few more comments for your perusal :)

I'm looking forward to having this merged!

rust/frontend/src/function.rs Outdated Show resolved Hide resolved
rust/frontend/src/function.rs Outdated Show resolved Hide resolved
rust/frontend/src/function.rs Outdated Show resolved Hide resolved
rust/frontend/src/function.rs Show resolved Hide resolved
rust/frontend/src/function.rs Outdated Show resolved Hide resolved
rust/frontend/src/internal_api.rs Outdated Show resolved Hide resolved
rust/frontend/src/module.rs Outdated Show resolved Hide resolved
rust/frontend/src/module.rs Outdated Show resolved Hide resolved
rust/frontend/src/module.rs Outdated Show resolved Hide resolved
rust/frontend/src/function.rs Outdated Show resolved Hide resolved
nhynes
nhynes reviewed Dec 17, 2018
View reviewed changes
rust/frontend/Cargo.toml Outdated Show resolved Hide resolved
@ehsanmok
Copy link
Contributor Author

ehsanmok commented Dec 17, 2018 •
edited

@nhynes @tqchen how come CI tvm-sys build is successful (tvm_runtime is found in ld path) as first dependency though tvm_frontend cannot find tvm_runtime?

This was referenced Dec 18, 2018
Closed
Closed
Closed
@nhynes
Copy link
Member

nhynes commented Dec 18, 2018

tvm_frontend cannot find tvm_runtime

In what context is this bug occurring? The most likely cause is that LD_LIBRARY_PATH simply becomes unset.

@ehsanmok
Copy link
Contributor Author

ehsanmok commented Dec 18, 2018 •
edited

@nhynes Please checkout task_rust.sh.

@nhynes
Copy link
Member

nhynes commented Dec 18, 2018

What happens if you also export LD_LIBRARY_PATH a la https://github.com/dmlc/tvm/blob/master/tests/scripts/task_java_unittest.sh#L3

@ehsanmok
Copy link
Contributor Author

ehsanmok commented Dec 18, 2018 •
edited

@nhynes Already exported in tests/scripts/task_rust.sh and I didn't change that.

@jroesch
Copy link
Member

jroesch commented Dec 18, 2018

I will take a pass over this later today.

@ehsanmok ehsanmok mentioned this pull request Dec 21, 2018
Closed
@ehsanmok ehsanmok mentioned this pull request Dec 24, 2018
Closed
5 tasks
@ehsanmok ehsanmok mentioned this pull request Jan 9, 2019
Merged
@ehsanmok
Copy link
Contributor Author

@nhynes pinging for the updated PR!

@nhynes
Copy link
Member

nhynes commented Jan 23, 2019

re-reviewing now!

nhynes
nhynes suggested changes Jan 23, 2019
View reviewed changes
Copy link
Member

@nhynes nhynes left a comment

Choose a reason for hiding this comment

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

nice! this is the last round of changes :)

rust/common/Cargo.toml Outdated Show resolved Hide resolved
rust/common/src/lib.rs Outdated Show resolved Hide resolved
rust/common/tvm-sys/Cargo.toml Outdated Show resolved Hide resolved
rust/frontend/Cargo.toml Outdated Show resolved Hide resolved
rust/frontend/Cargo.toml Outdated Show resolved Hide resolved
rust/frontend/src/context.rs Outdated Show resolved Hide resolved
rust/frontend/src/function.rs Outdated Show resolved Hide resolved
rust/frontend/src/ty.rs Show resolved Hide resolved
rust/frontend/src/ty.rs Outdated Show resolved Hide resolved
rust/runtime/src/errors.rs Outdated Show resolved Hide resolved
@nhynes nhynes mentioned this pull request Jan 23, 2019
Merged
@ehsanmok ehsanmok mentioned this pull request Jan 25, 2019
Closed
nhynes
nhynes suggested changes Jan 28, 2019
View reviewed changes
Copy link
Member

@nhynes nhynes left a comment

Choose a reason for hiding this comment

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

I pulled the branch and made some changes w.r.t. returning references instead of sprinkling in to_owneds

rust/frontend/src/module.rs Outdated Show resolved Hide resolved
@nhynes
Copy link
Member

nhynes commented Jan 28, 2019

We're going to need to work on error messages

thread 'main' panicked at '', tvm_rust/rust/frontend/src/function.rs:262:13
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

isn't a very helpful output of checked_call! :(

@ZihengJiang
Copy link
Contributor

@nhynes The jenkins image has been updated. Please submit an empty commit to retrigger the CI

@tqchen
Copy link
Member

tqchen commented Feb 2, 2019

Sorry, I have to revert the image due to a short outrage to recover ANTLR, will send in update

@nhynes nhynes merged commit e2970b2 into apache:master Feb 3, 2019
libing4752 pushed a commit to libing4752/tvm that referenced this pull request Feb 18, 2019
@ehsanmok @libing4752
[RUST][FRONTEND] Add rust frontend v0.1 (apache#2292)
c210368
merrymercy pushed a commit to merrymercy/tvm that referenced this pull request Feb 18, 2019
@ehsanmok @merrymercy
[RUST][FRONTEND] Add rust frontend v0.1 (apache#2292)
8008fd0
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
@ehsanmok
[RUST][FRONTEND] Add rust frontend v0.1 (apache#2292)
27deca7
wweic pushed a commit to neo-ai/tvm that referenced this pull request Feb 20, 2019
@ehsanmok
[RUST][FRONTEND] Add rust frontend v0.1 (apache#2292)
adcc5c2
@yzhliu yzhliu mentioned this pull request Mar 2, 2019
Closed
28 tasks
@tqchen tqchen mentioned this pull request Nov 8, 2019
Closed
@ehsanmok ehsanmok deleted the rust_frontend branch April 27, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nhynes nhynes nhynes 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.

[RUST] RFC and 0.5 Release Plans
6 participants
@ehsanmok @nhynes @jroesch @ZihengJiang @tqchen @icemelon