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

Expose llvm.nearbyint intrinsic. This is a faster alternate to rounding. #4001

Merged
merged 2 commits into from
Sep 25, 2019

Conversation

kimishpatel
Copy link
Contributor

In the quantized gemm implementation we are working on, we need to quantize input data. During this step we first apply scale and zero point to the input data. Then we do rounding and casting to int8.

tvm::round gets lowered by llvm into roundf function call which make the op slower. I instead exposed llvm.nearbyint via tvm and was able to recover the lost performance.

So this PR is just upstreaming that change.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@kimishpatel
Copy link
Contributor Author

@tqchen, sorry have to tag you again on this PR as it seems that you may be the right person. Please feel free to suggest anyone else, but this is a quite small PR.

Copy link
Member

@tqchen tqchen left a comment

Choose a reason for hiding this comment

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

Please add a python wrapper, a reference to the docs in the https://github.com/dmlc/tvm/tree/master/docs/api/python, and a testcase

@tqchen tqchen added status: need test case need test cases to cover the change status: need update need update based on feedbacks labels Sep 25, 2019
@tqchen
Copy link
Member

tqchen commented Sep 25, 2019

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@kimishpatel
Copy link
Contributor Author

Thanks @tqchen, I have added test case and python binding. Thanks for your feedback.

@anijain2305
Copy link
Contributor

anijain2305 commented Sep 25, 2019

@kimishpatel Thanks for the PR. You can also look at qnn.op.quantize. This is a wrapper that internally lowers to the sequence of relay ops you mentioned. This PR will benefit that wrapper as well.

@kimishpatel
Copy link
Contributor Author

@anijain2305, thanks for the pointer. Wasn't quite aware of that.

@anijain2305
Copy link
Contributor

@anijain2305, thanks for the pointer. Wasn't quite aware of that.

If you are looking at running pre quantized models, you might want to have a look at QNN dialect. We have added a number of QNN ops in there that deal with scale and zero points.

@kimishpatel
Copy link
Contributor Author

@tqchen, sorry to bug again :). Seems like all checks have passed, so just nudging for the merge. Thanks a bunch.

@tqchen tqchen merged commit 17c2c0a into apache:master Sep 25, 2019
@tqchen
Copy link
Member

tqchen commented Sep 25, 2019

Thanks @anijain2305 @kimishpatel

@tqchen tqchen added status: accepted and removed status: need test case need test cases to cover the change status: need update need update based on feedbacks labels Sep 25, 2019
wweic pushed a commit to wweic/tvm that referenced this pull request Sep 30, 2019
…ng. (apache#4001)

* Expose llvm.nearbyint intrinsic. This is a faster alternate to rounding.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

* Added python binding. Added test.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
wweic pushed a commit to wweic/tvm that referenced this pull request Sep 30, 2019
…ng. (apache#4001)

* Expose llvm.nearbyint intrinsic. This is a faster alternate to rounding.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

* Added python binding. Added test.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
wweic pushed a commit to neo-ai/tvm that referenced this pull request Oct 1, 2019
…ng. (apache#4001)

* Expose llvm.nearbyint intrinsic. This is a faster alternate to rounding.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

* Added python binding. Added test.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
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