[RUNTIME][GOLANG] TVM runtime for golang v0.1#1470
Conversation
|
It is a good start, I would recommend to not use swig but directly use the C FFI in Go, and do a bit wrapping to support Module and TVMValue class like in jvm, |
|
Ref. gomxnet above Reason I choose swig was to keep the interface maintenance automatic for changes to c_runtime_api. pls advice. |
|
I think doing a bit minimum wrapping to make things more native to the language(in this case go) is useful, Take a look at tvm4j and the wrapping being done there |
|
I checked the tvm4j and tried to wrap few types inside TVMvalue. I see other challenges in wrapping the go slices to C arrays for TVMValue array itself and other primitive types. golang use slices very common instead of arrays which complicates the interface. Hand coding slice to a array for all these types via "C" and "unsafe" need more effort and results almost same like the swig generated interface. Please advice. |
|
I think if we can build an NDArray.copyfrom(go_slice) function, it will work for most case. The data conversion happens inside the function itself, so it is invisible to the users. BTW, in terms of package name, seems gotvm is a more canonical name for golang projects :) |
|
@tqchen please have a look. @dmlc/tvm-reviewer welcome to review and comment. |
|
@srkreddy1238 Can you do additional wrappings to provide the following class object?
|
|
Sure, I will add above class objects. |
|
Also please add appropriate documentation for user-facing objects and methods, as per https://blog.golang.org/godoc-documenting-go-code |
|
|
@tqchen ready for review. Recent changes
|
| @@ -0,0 +1,51 @@ | |||
| # tvmgo - Golang Frontend for TVM Runtime | |||
| #include <tvm/runtime/c_runtime_api.h> | ||
| #include <dlpack/dlpack.h> | ||
|
|
||
| // Some type devinitions for golang "C" |
There was a problem hiding this comment.
provide wrapping in c/c++ via extern "C" and __cplusplus
There was a problem hiding this comment.
gotvm.h ideally should be a command followed by import "C"
cgo picks up picks it it from commented code.
No problem, extern works here well.
Done
| @@ -0,0 +1,292 @@ | |||
| /*! | |||
| * Copyright (c) 2018 by Contributors | |||
| * \brief gotvm native interface definition | |||
There was a problem hiding this comment.
unless it is the convention, use gotvm.cc
|
I made some simple comments, but the wrapping is still a bit raw(it reads more like C rather than Go), here is what we want ideally(note I am not expert in Go, so syntax might be wrong) mod = gotvm.LoadFromFile("/path/to/mymodule.so")
f1 = mod.GetFunction("myfunction")
# need to debate how to support function invocation, either in normal style or java style
f1.invoke(args)
myarr = gotvm.empty(shape, ctx); |
|
Actually since, this involves API design discussion, can you please open an RFC issue so that more people can participate? Maybe there are good ideas from the community, I am not expert in Go, so I can only generalize from my experience in other languages |
|
#1559 for a discussion with team. |
f1 = mod.GetFunction("myfunction")
f1.invoke(args)
f1.free()The new API |
719b1e9 to
ae25688
Compare
* Value to use finalizers. * README updated.
|
Thanks @grwlf. Addressed the review comments. |
|
OK, if there is no further comments, I will merge this PR in 24 hours. Thanks @grwlf for the reviews. |
|
Thanks, @grwlf for the detailed reviews, Thanks, @srkreddy1238 for patiently improving the code during the review process. This is now merged. Please send in followup PRs to enable the test in CI |
Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from others in the community.