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

Vulkan TVM Android Support #1571

Merged
merged 3 commits into from
Aug 10, 2018
Merged

Conversation

Dayananda-V
Copy link
Contributor

  1. Vulkan Tvm Context added
  2. Vulkan test case for rpc added

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.

1. Vulkan Tvm Context added
2. Vulkan test case for rpc added
@Dayananda-V
Copy link
Contributor Author

@yzhliu

Welcome for review.

@Dayananda-V Dayananda-V closed this Aug 8, 2018
@Dayananda-V Dayananda-V reopened this Aug 8, 2018
@tqchen
Copy link
Member

tqchen commented Aug 8, 2018

@eqy can you also take a look?

@tqchen tqchen self-assigned this Aug 8, 2018
cost = time_f(a, b).mean
print('%g secs/op' % cost)
np.testing.assert_equal(b.asnumpy(), a.asnumpy() + 1)

print('Run CPU test ...')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we tweak the ordering of the tests so that the CPU test runs first? This provides some easy catching of common failure modes (e.g., the user did not link OpenCL so the CPU test runs but the GPU test fails).

@@ -21,59 +21,86 @@
arch = "arm64"
target = "llvm -target=%s-linux-android" % arch

test_opencl = True
test_vulkan = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider making these required command line options so we do not have broken defaults depending on the user's configuration (e.g., a CPU-only android device). We should also update the Android install docs to describe the test options (CPU, OpenCL, Vulkan).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, will work on comment

1. Review comment reworked.
@Dayananda-V
Copy link
Contributor Author

Thanks @eqy

reworked on review comment.

1. Review comment reworked.
@tqchen tqchen merged commit 2afe024 into apache:master Aug 10, 2018
@tqchen
Copy link
Member

tqchen commented Aug 10, 2018

Thanks @dayanandasiet @eqy , this is merged!

@tqchen tqchen mentioned this pull request Aug 10, 2018
FrozenGene pushed a commit to FrozenGene/tvm that referenced this pull request Dec 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants