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.

Sign up for GitHub

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

Jump to bottom

[BugFix][UMA] Protect target registration #13624

Merged
merged 1 commit into from
Dec 16, 2022
Merged

[BugFix][UMA] Protect target registration #13624

merged 1 commit into from
Dec 16, 2022

Conversation

cbalint13
Copy link
Contributor

@cbalint13 cbalint13 commented Dec 15, 2022 •
edited
Loading

This PR address fixes for UMA target registration.

Changes:

  • Incorporates all proposed fixes from mentioned PR #12731
  • Address test case concerns and discussions from PR #12731
  • NEW: Already exiting target cannot be created, explicit error on this.
  • NEW: Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

Cc: @areusch, @cgerum, @lhutton1, @manupak, @MichaelJKlaiber, please help with the review.

Thanks,
~Cristian

@tvm-bot
Copy link
Collaborator

tvm-bot commented Dec 15, 2022

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

  • No users to tag found in teams: bugfix, uma See #10317 for details

Generated by tvm-bot

@cbalint13 cbalint13 changed the title [BugFix][UMA] Protect target registration. [BugFix][UMA] Protect target registration Dec 15, 2022
@cgerum
Copy link
Contributor

cgerum commented Dec 15, 2022

Thank you very much @cbalint13 for taking over some UMA maintenance. The patch looks fine to me.

manupak
manupak approved these changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@manupak manupak left a comment

Choose a reason for hiding this comment

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

Thanks for adding the tests! Just one nit.

src/relay/backend/contrib/uma/targets.cc Outdated Show resolved Hide resolved
lhutton1
lhutton1 approved these changes Dec 16, 2022
View reviewed changes
Copy link
Contributor

@lhutton1 lhutton1 left a comment

Choose a reason for hiding this comment

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

Thanks for picking these up @cbalint13, LGTM!

@lhutton1 lhutton1 merged commit 0eabbac into apache:main Dec 16, 2022
@lhutton1
Copy link
Contributor

Thanks @cbalint13, @cgerum, @manupak!

This was referenced Dec 16, 2022
Closed
Closed
@lhutton1
Copy link
Contributor

lhutton1 commented Dec 16, 2022 •
edited
Loading

@leandron, could this be a candidate for cherry-picking in the v0.11 release? (#13586)

@leandron
Copy link
Contributor

leandron commented Jan 3, 2023

@leandron, could this be a candidate for cherry-picking in the v0.11 release? (#13586)

Yes will do.

leandron pushed a commit to leandron/tvm that referenced this pull request Jan 19, 2023
@cbalint13 @leandron
[BugFix][UMA] Protect target registration (apache#13624)
da30a4a 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304
* Continues stalled PR apache#12731

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

(cherry picked from commit 0eabbac)
@leandron leandron mentioned this pull request Jan 19, 2023
Open
7 tasks
leandron pushed a commit to leandron/tvm that referenced this pull request Feb 2, 2023
@cbalint13 @leandron
[BugFix][UMA] Protect target registration (apache#13624)
2f999d6 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304 
* Continues stalled PR apache#12731 

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.
tqchen pushed a commit to tqchen/tvm that referenced this pull request Feb 20, 2023
@cbalint13 @tqchen
[BugFix][UMA] Protect target registration (apache#13624)
e0ebf28 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304 
* Continues stalled PR apache#12731 

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.
@tqchen tqchen mentioned this pull request Feb 20, 2023
Merged
tqchen pushed a commit to tqchen/tvm that referenced this pull request Feb 20, 2023
@cbalint13 @tqchen
[BugFix][UMA] Protect target registration (apache#13624)
50b33ac 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304
* Continues stalled PR apache#12731

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

Signed-off-by: tqchen <[email protected]>
tqchen pushed a commit to tqchen/tvm that referenced this pull request Feb 20, 2023
@cbalint13 @tqchen
[BugFix][UMA] Protect target registration (apache#13624)
8141744 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304
* Continues stalled PR apache#12731

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

Signed-off-by: tqchen <[email protected]>
tqchen added a commit to tqchen/tvm that referenced this pull request Feb 20, 2023
@tqchen @cbalint13
[BugFix][UMA] Protect target registration (apache#13624)
a2c01ac 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304
* Continues stalled PR apache#12731

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

Co-authored-by: Balint Cristian <[email protected]>
tqchen added a commit to tqchen/tvm that referenced this pull request Feb 20, 2023
@tqchen @cbalint13
[BugFix][UMA] Protect target registration (apache#13624) (#2)
7002557 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304
* Continues stalled PR apache#12731

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

Signed-off-by: tqchen <[email protected]>
Co-authored-by: Balint Cristian <[email protected]>
tqchen added a commit to tqchen/tvm that referenced this pull request Feb 20, 2023
@tqchen @cbalint13
[BugFix][UMA] Protect target registration (apache#13624) (#3)
e5906e1 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304
* Continues stalled PR apache#12731

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

Signed-off-by: tqchen <[email protected]>
Co-authored-by: Balint Cristian <[email protected]>
tqchen added a commit to tqchen/tvm that referenced this pull request Feb 20, 2023
@tqchen @cbalint13
[BugFix][UMA] Protect target registration (apache#13624) (#4)
8c1361e 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304
* Continues stalled PR apache#12731

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

Signed-off-by: tqchen <[email protected]>
Co-authored-by: Balint Cristian <[email protected]>
tqchen added a commit that referenced this pull request Feb 20, 2023
@tqchen @cbalint13
[BugFix][UMA] Protect target registration (#13624) (#14057)
9bc6532 
[BugFix][UMA] Protect target registration (#13624) (#4)

This PR address fixes for UMA target registration.
* Fix the doc issue #13304
* Continues stalled PR #12731

Changes:
* Incorporates all proposed fixes from mentioned [PR #12731](#12731)
* Address test case concerns and discussions from [PR #12731](#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.

Signed-off-by: tqchen <[email protected]>
Co-authored-by: Balint Cristian <[email protected]>
fzi-peccia pushed a commit to fzi-peccia/tvm that referenced this pull request Mar 27, 2023
@cbalint13 @fzi-peccia
[BugFix][UMA] Protect target registration (apache#13624)
54358e0 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304 
* Continues stalled PR apache#12731 

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.
@ysh329 ysh329 mentioned this pull request Apr 17, 2023
Closed
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
@cbalint13
[BugFix][UMA] Protect target registration (apache#13624)
6e56bb4 
This PR address fixes for UMA target registration.
* Fix the doc issue apache#13304 
* Continues stalled PR apache#12731 

Changes:
* Incorporates all proposed fixes from mentioned [PR apache#12731](apache#12731)
* Address test case concerns and discussions from [PR apache#12731](apache#12731)
* **NEW:** Already exiting target cannot be created, explicit error on this.
* **NEW:** Attributes having special/reserved scope cannot be created explicitly.

It also address proper test cases for all the above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lhutton1 lhutton1 lhutton1 approved these changes

@manupak manupak manupak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@cbalint13 @tvm-bot @cgerum @lhutton1 @leandron @manupak