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

Fix maxGridSizes were reported negative #23

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix maxGridSizes were reported negative #23

wants to merge 2 commits into from

Conversation

linehill
Copy link
Collaborator

@linehill linehill commented Jun 7, 2021

  • Fix maxGridSizes values from hipGetDeviceProperties() were reported negative. More details in the commit log.

  • Add a dedicated test directory and a regression test for this issue.

Henry Linjamäki added 2 commits June 7, 2021 11:54
Fix hipGetDeviceProperties() reported negative maxGridSizes
31c2228 
This occurred because on my machine the level zero API reported 2^31+
values for the corresponding properties which were then
unintentionally converted to negative number because HIP API uses int
type for them.

Fix by clamping the properties to [0, INT_MAX].
Add a test directory and a regression test
6faf5ca 
* Add a directory for testing purposes.

* Add a regression test for maxGridSize issue.
@Kerilk
Copy link
Collaborator

Kerilk commented Jun 7, 2021 •
edited
Loading

Thanks for all the great contributions.
I am going to merge this one and then merge master into our development branch (wip). I will also exceptionally merge back the development branch into master (We usually only do that for MS delivery) because of this merge request that was accepted recently: #21
The goal of the merge request is to help upstream back-end development by clearly labeling everything as HIPLZ or hiplz (library included) in order to have HIPCL and HIPLZ be able to live side by side without conflicts. This changed a lot of macros and variables that will affect subsequent pull requests.
My goal is also trying to define the precise frontiers of what could be shared between HIPCL and HIPLZ. I was thinking the device bitcode and the extra llvm passes could go in a third repository that would be used as a submodule, as they are SPIRV specific and not backend specific.
Tell me what you think.
Tagging @pjaaskel as well as this has a lot of ramifications.

@linehill
Copy link
Collaborator Author

linehill commented Jun 8, 2021

IHMO, I think it wouldn't be a big issue if the bitcode and the LLVM passes were duplicated among HIPCL and HIPZL repositories. The current state might be a bit better in case any backend specific changes are needed in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@linehill @Kerilk