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

[Hexagon] Treat floats as float32 when passing args to offloaded kernels #9010

Merged
merged 1 commit into from
Sep 15, 2021
Merged

[Hexagon] Treat floats as float32 when passing args to offloaded kernels #9010

merged 1 commit into from
Sep 15, 2021

Conversation

kparzysz-quic
Copy link
Contributor

TVMArg can hold a floating point value, but it's stored as double. In Hexagon ABI doubles are passed in a register pair, but if the offloaded function was using floats (i.e. float32), it will expect values being passed in single registers. Since floats are much more common on Hexagon, assume all scalar floating point values are floats. This is only an issue with offloading, and can be treated as a limitation (we do something analogous for integers already).

`TVMArg` can hold a floating point value, but it's stored as `double`. In
Hexagon ABI doubles are passed in a register pair, but if the offloaded
function was using floats (i.e. float32), it will expect values being
passed in single registers. Since floats are much more common on Hexagon,
assume all scalar floating point values are floats. This is only an issue
with offloading, and can be treated as a limitation (we do something
analogous for integers already).
Copy link
Contributor

@tmoreau89 tmoreau89 left a comment

Choose a reason for hiding this comment

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

Thanks LGTM, although I'd like a second take from @areusch

@tmoreau89 tmoreau89 merged commit 57386a2 into apache:main Sep 15, 2021
@tmoreau89
Copy link
Contributor

Thank you @kparzysz-quic - the PR has been merged

@kparzysz-quic kparzysz-quic deleted the hexagon-offload-float32 branch September 15, 2021 15:35
AndrewZhaoLuo added a commit to AndrewZhaoLuo/tvm that referenced this pull request Sep 16, 2021
* main: (102 commits)
  Implementation of relay_to_tir target hook (apache#8423)
  [Onnx] Fix NLL Loss tests (apache#8971)
  [Bugfix] Fix other div zero errors also in rewrite_simplify (apache#8983)
  [ONNX] enable the onnx tests after PR apache#8274 merged (apache#9019)
  [Hexagon] Disable `thread_local` on Hexagon (apache#9025)
  [Hexagon] Allow undefined symbols in libtvm_runtime.so on Hexagon (apache#9024)
  [Onnx] Add momentum (apache#9000)
  fix (apache#9021)
  [Community] @AndrewZhaoLuo -> Reviewer (apache#9020)
  [Hexagon] Implement model launcher (apache#8986)
  [Relay][Pass] Add ExtractOperators pass (apache#8996)
  [BYOC][TensorRT] Add TensorRT own int8 calibration support to TensorRT BYOC integration (apache#8808)
  [ONNX] Add Einsum converter (apache#8985)
  Add standalone_crt/ to be part of the wheel package, when available. (apache#9005)
  [Relay] Remove memory planing from LowerTEPass  (apache#8974)
  [Hexagon] Treat floats as float32 when passing args to offloaded kernels (apache#9010)
  [Runtime] Pipeline Executor Initial patch. (apache#8702)
  [Hexagon] `llvm-options` attribute is an array of strings (apache#9011)
  disable cuda int8 schedule for non-cuda gpu target (apache#9014)
  [Torch] Add an option to make imported models compatible with the Relay text parser (apache#9015)
  ...
@areusch
Copy link
Contributor

areusch commented Sep 20, 2021

sorry i missed this @tmoreau89 @kparzysz-quic . i think this makes sense for now. it seems like in the post-fused-IRModule world, we could do some analysis of the calling/called function to eliminate this limitation. what do you think? also, seems like we should potentially document the TVM port in a README/docs.tvm.ai somewhere. what are your thoughts about that?

ylc pushed a commit to ylc/tvm that referenced this pull request Sep 29, 2021
…els (apache#9010)

`TVMArg` can hold a floating point value, but it's stored as `double`. In
Hexagon ABI doubles are passed in a register pair, but if the offloaded
function was using floats (i.e. float32), it will expect values being
passed in single registers. Since floats are much more common on Hexagon,
assume all scalar floating point values are floats. This is only an issue
with offloading, and can be treated as a limitation (we do something
analogous for integers already).
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
…els (apache#9010)

`TVMArg` can hold a floating point value, but it's stored as `double`. In
Hexagon ABI doubles are passed in a register pair, but if the offloaded
function was using floats (i.e. float32), it will expect values being
passed in single registers. Since floats are much more common on Hexagon,
assume all scalar floating point values are floats. This is only an issue
with offloading, and can be treated as a limitation (we do something
analogous for integers already).
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.

3 participants