[RUNTIME] Initial implementation of Hexagon runtime support #5252

kparzysz-quic · 2020-04-06T17:26:47Z

This is only the TVM runtime. The FastRPC libraries, simulator driver, etc. will be provided in subsequent commits.

tqchen · 2020-04-06T18:01:23Z

tmoreau89

Great work, awesome to see a major new runtime being added to TVM. I left a couple comments.

tmoreau89 · 2020-04-06T18:08:48Z

src/runtime/hexagon/README.md

+
+The TVM runtime that contains Hexagon runtime is the one executing on host.  In either case, there will need to be a separate TVM runtime (i.e.  the `libtvm_runtime.so` library) compiled for execution on Hexagon.
+
+The prerequisite is to have Hexagon SDK installed, preferably version 3.5.0.


Could you add a link to installing the Hexagon SDK?

In addition, is there a way to install it "automatically" to this back-end can be tested in the CI?

Sure, I'll add the link. I'm not sure about automatic install, because there is a license that needs to be accepted during installation.

While it might be hard to create an automatic installed version, we can look into at least do the compilation tests, to make sure that the runtime compiles

The SDK link is https://developer.qualcomm.com/software/hexagon-dsp-sdk. I'll update the PR once the build completes (I want to see if it passes)...

include/tvm/runtime/device_api.h

tmoreau89

Thanks for the PR, and addressing the comments, the changes LGTM.

FrozenGene · 2020-04-07T03:53:25Z

src/runtime/hexagon/target/hexagon_device_target.cc

+  TVM_LOGD_HT("Done VTCM free from HexagonTarget::FreeVtcm");
+}
+
+void HexagonTarget::CopyDeviceToDevice(void* dst, const void* src,


As comment said: #3163 (comment) We should implement it. I tested on real device and find it is one must for device.

FrozenGene · 2020-04-07T04:10:30Z

src/runtime/hexagon/target/hexagon_device_target.cc

+  const DspRpcAPI* dsp_api = DspRpcAPI::Global();
+  const StubAPI* stub_api = StubAPI::Global();
+
+  stub_api->rpcmem_init_ptr()();


I maybe forget a little bit, because I do it last year. However, I remember rpcmem_init_ptr will have problem when we set use_unsigned_pd be false. In your code logic of this pr, you will always set use_unsigned_pd be true. Could you double check it? I set it be false, because I find when we set it be true, we can not do parallel on DSP. But I don't have device now and can not verify it sadly.

I can set it to false, would you prefer that? I don't think there is a way to make it a parameter that the user can provide at run time.

Yes. I prefer setting it to be false.

FrozenGene · 2020-04-07T04:13:22Z

Could you provide the .idl file too? Because you have provided tvm_hexagon_remote.h, if we don't have .idl we can not generate .cc / .h for hexagon. I think it is core part of Hexagon runtime support too.

kparzysz-quic · 2020-04-07T13:38:09Z

The IDL file and the implementation of the Hexagon-side target code will be upstreamed in an upcoming PR.

tmoreau89

Thanks @kparzysz-quic , the changes LGTM

FrozenGene · 2020-04-08T09:46:31Z

src/runtime/hexagon/README.md

+USE_HEXAGON_SDK=/path/to/sdk
+```
+
+Set the C/C++ compiler to `clang`, and pass `-DCMAKE_CXX_FLAGS='-stdlib=libc++'` to the cmake command.


Does it a must? I use g++ and libstdc++ ever and no problem.

When we started, I was seeing some problems with libstdc++, but I don't remember what they were anymore. Since it worked for you, I removed this line.

FrozenGene · 2020-04-08T10:02:50Z

src/runtime/hexagon/sim/hexagon_device_sim.cc

+  CHECKED_CALL(ConfigureAppCommandLine, sim_dev_args_.size(), sim_dev_args_);
+
+  LOG(INFO) << "HexagonSimulator: ConfigureHVXLength: 128";
+  CHECKED_CALL(ConfigureHVXLength, 128);


I am concerning here. When we create Hexagon target and use HVX 64, but we write hard code of 128 here. We can not find the problem easily. I didn't think of good solution before too. If we don't have a better way, could we add comment in def hexagon(cpu_ver='v66', sim_args=None, hvx=128): to prompt users if you change the hvx length, we should change the simulator here?

This was actually only setting the default HVX length. If --hvx_length was specified in sim_args, it would be processed in Configure` below, resetting the HVX length to the new value.

I changed the processing of sim_args to always add --hvx_length that matches hvx (it already printed a warning if both were given and didn't match). Now, configuring the default HVX length here is not needed, so I removed it.

FrozenGene · 2020-04-08T10:11:49Z

src/runtime/hexagon/hexagon_module.cc

+
+std::shared_ptr<Device> Device::Global() {
+  // Declare device constructors.
+#ifdef __ANDROID__


If we want to support more Hexagon target in the future, we maybe have problem. For example, hexagon could run embed system of Ubuntu, which is official support of Hexagon SDK too. Then we can not use __ANDROID__ to distinguish target or simulator. One proposal is in our Hexagon.cmake, when we build for simulator, we define one macro like -D__HEXAGON_SIM__=1 to indicate we are running on simulator or like -D__HEXAGON_DEVICE__=1 to indicate we are running on real devices. This is one nit-pick comment, however, I wish we could consider it.

We can probably do that, but I'd prefer to do it in another commit.

FrozenGene · 2020-04-09T03:45:32Z

src/runtime/hexagon/README.md

+
+When configuring TVM (cmake), set the following variables:
+```
+USE_LLVM=llvm-config


Need us to add one line of LLVM version requirement? If I remember correctly, older LLVM version (like 6.0 or before) has many bugs on Hexagon DSP.

Done. I'm not aware of any specific issues with older LLVMs for Hexagon, so I didn't know which version is the minimum. I used 7.0.0 as the minimum, based on your comment, but recommended using as recent version as possible.

FrozenGene · 2020-04-09T03:46:49Z

LGTM except one last comment. @kparzysz-quic Could you fix the CI?

kparzysz-quic · 2020-04-09T14:01:58Z

I don't think that the CI failure has anything to do with this PR, there are other PRs that failed in the same way. I rebased it op top of the current master, maybe that will help...

kparzysz-quic · 2020-04-09T14:39:01Z

Example of two other PRs that fail in the same place:
https://ci.tvm.ai/blue/organizations/jenkins/tvm/detail/PR-5272/8/pipeline/152
https://ci.tvm.ai/blue/organizations/jenkins/tvm/detail/PR-5276/6/pipeline/152

This is only the TVM runtime. The FastRPC libraries, simulator driver, etc. will be provided in subsequent commits.

Apparently things work with libstdc++.

kparzysz-quic · 2020-04-10T12:11:55Z

@FrozenGene If you're happy with the code, could you approve? Your review is still showing as "requested changes".

FrozenGene · 2020-04-10T13:50:25Z

@kparzysz-quic Thanks for the great work. Looking forward other parts of Hexagon like IDL / codegen part and so on. Thanks @tmoreau89 for the code review!

) * [RUNTIME] Initial implementation of Hexagon runtime support This is only the TVM runtime. The FastRPC libraries, simulator driver, etc. will be provided in subsequent commits. * Fix pylint complaints * Fix some more pylint complaints * Add link to the Hexagon SDK website * Extract VTCM marker into a common variable * Implement device->device memory copy * Disable unsigned PDs by default * Ensure that --hvx_length is present in sim_args if HVX is enabled * Remove the line about clang from README.md Apparently things work with libstdc++. * Mention to set USE_RPC=OFF when building libtvm_runtime.so for Hexagon * Remember to use codegen_hvx in validate_hvx_length * Add a line about minimum version of LLVM

…m_data:master to master * commit 'cd0d52daa6942bdafa9363ff6cfa3d25fcd5b8d6': (824 commits) [Intrinsic] Add log1p, ldexp, atan2, hypot, nextafter, copysign (apache#5312) [Rust][CI] Restore Rust CI (apache#5137) Remove PrimExpr from String (apache#5311) [Requantize] Cleanup and Optimize Lowering (apache#5286) [IR][TRANSFORM] Enable CopyOnWrite for passes. (apache#5309) [PYTORCH]Abs, Arange, Softplus ops (apache#5295) [LLVM] Fix generation of LLVM intrinsics (apache#5282) [BYOC] Add example of Composite + Annotate for DNNL fused op (apache#5272) [Frontend][TensorFlow]Improve TensorFlow Static Shape Tensor Array (apache#5243) [RUNTIME] Introduce RValue reference(move) support to TypedPackedFunc (apache#5271) [RELAY][FRONTEND][CAFFE2] add Mul and ConvTranspose operator (apache#5302) [BYOC] Refine AnnotateTarget and MergeCompilerRegion Passes (apache#5277) [CI] Fix the hexagon string (apache#5304) [Arith] linear system and equation solver (apache#5171) [PYTORCH]Repeat, Reciprocal & Reshape Op support (apache#5280) [FRONTEND][TENSORFLOW] Fix gather_nd indices (apache#5279) Update device_annotation.cc (apache#5291) [REFACTOR][IR] Move to runtime::String (apache#5276) [NDArray] Set shape_ in NDArray::FromDLPack (apache#5301) [RUNTIME] Initial implementation of Hexagon runtime support (apache#5252) ...

kparzysz-quic mentioned this pull request Apr 6, 2020

[RUNTIME] Initial implementation of Hexagon runtime support #3163

Closed

tqchen added the status: need review label Apr 6, 2020

tmoreau89 reviewed Apr 6, 2020

View reviewed changes

tmoreau89 approved these changes Apr 6, 2020

View reviewed changes

FrozenGene requested changes Apr 7, 2020

View reviewed changes

tqchen assigned tmoreau89 and FrozenGene Apr 7, 2020

tmoreau89 approved these changes Apr 7, 2020

View reviewed changes

FrozenGene reviewed Apr 8, 2020

View reviewed changes

FrozenGene reviewed Apr 9, 2020

View reviewed changes

Krzysztof Parzyszek added 12 commits April 9, 2020 20:31

[RUNTIME] Initial implementation of Hexagon runtime support

bcf6a9f

This is only the TVM runtime. The FastRPC libraries, simulator driver, etc. will be provided in subsequent commits.

Fix pylint complaints

6b542e0

Fix some more pylint complaints

ca3f9d1

Add link to the Hexagon SDK website

7fc718c

Extract VTCM marker into a common variable

7d7ca2c

Implement device->device memory copy

19927d4

Disable unsigned PDs by default

8bd81d1

Ensure that --hvx_length is present in sim_args if HVX is enabled

e6b623d

Remove the line about clang from README.md

701b2f1

Apparently things work with libstdc++.

Mention to set USE_RPC=OFF when building libtvm_runtime.so for Hexagon

8e879f3

Remember to use codegen_hvx in validate_hvx_length

52de89a

Add a line about minimum version of LLVM

2296b94

FrozenGene approved these changes Apr 10, 2020

View reviewed changes

FrozenGene merged commit 02d3a59 into apache:master Apr 10, 2020

kparzysz-quic deleted the hexagon.rt branch April 10, 2020 14:52

tqchen added status: accepted and removed status: need review labels Apr 13, 2020

ZihengJiang mentioned this pull request Sep 25, 2020

TVM v0.7 Release Note Candidate #6486

Closed

This was referenced Aug 23, 2022

[Runtime] Change default alignment to 64 bytes. #12564

Closed

[Runtime] Change default alignment to 64 bytes. #12586

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RUNTIME] Initial implementation of Hexagon runtime support #5252

[RUNTIME] Initial implementation of Hexagon runtime support #5252

kparzysz-quic commented Apr 6, 2020

tqchen commented Apr 6, 2020

tmoreau89 left a comment

tmoreau89 Apr 6, 2020

tmoreau89 Apr 6, 2020

kparzysz-quic Apr 6, 2020

tqchen Apr 6, 2020

kparzysz-quic Apr 6, 2020

tmoreau89 Apr 6, 2020

tmoreau89 left a comment

FrozenGene Apr 7, 2020

kparzysz-quic Apr 7, 2020

FrozenGene Apr 7, 2020

kparzysz-quic Apr 7, 2020

FrozenGene Apr 7, 2020

kparzysz-quic Apr 7, 2020

FrozenGene commented Apr 7, 2020

kparzysz-quic commented Apr 7, 2020

tmoreau89 left a comment

FrozenGene Apr 8, 2020

kparzysz-quic Apr 8, 2020

FrozenGene Apr 8, 2020

kparzysz-quic Apr 8, 2020

FrozenGene Apr 8, 2020 •

edited

Loading

kparzysz-quic Apr 8, 2020

FrozenGene Apr 9, 2020

kparzysz-quic Apr 9, 2020

FrozenGene commented Apr 9, 2020

kparzysz-quic commented Apr 9, 2020

kparzysz-quic commented Apr 9, 2020

kparzysz-quic commented Apr 10, 2020

FrozenGene commented Apr 10, 2020


		The TVM runtime that contains Hexagon runtime is the one executing on host. In either case, there will need to be a separate TVM runtime (i.e. the `libtvm_runtime.so` library) compiled for execution on Hexagon.

		The prerequisite is to have Hexagon SDK installed, preferably version 3.5.0.

[RUNTIME] Initial implementation of Hexagon runtime support #5252

[RUNTIME] Initial implementation of Hexagon runtime support #5252

Conversation

kparzysz-quic commented Apr 6, 2020

tqchen commented Apr 6, 2020

tmoreau89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmoreau89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrozenGene commented Apr 7, 2020

kparzysz-quic commented Apr 7, 2020

tmoreau89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrozenGene Apr 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FrozenGene commented Apr 9, 2020

kparzysz-quic commented Apr 9, 2020

kparzysz-quic commented Apr 9, 2020

kparzysz-quic commented Apr 10, 2020

FrozenGene commented Apr 10, 2020

FrozenGene Apr 8, 2020 •

edited

Loading