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

Experimental AMD extensions #184

Merged
merged 1 commit into from
Feb 25, 2019
Merged

Experimental AMD extensions #184

merged 1 commit into from
Feb 25, 2019

Conversation

msiglreith
Copy link
Contributor

Addresses #178 (comment)

Not fully supports all parts of the APIs, just trying to get some general feedback if this meets the expected structuring.

Copy link
Member

@MaikKlein MaikKlein left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. Do you also want to do the higher level loader? Also I just realized that if we split ash into ash-sys then we can't easily extend StructureType like this any more.

}
}
impl<'a> PhysicalDeviceWaveLimitPropertiesAmdBuilder<'a> {
pub fn next<T>(mut self, next: &'a mut T) -> PhysicalDeviceWaveLimitPropertiesAmdBuilder<'a>
Copy link
Member

Choose a reason for hiding this comment

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

I believe this should be &'a T right now, but if #183 merges then this is correct.

Copy link
Member

Choose a reason for hiding this comment

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

Just leave it as it is right now.

@Ralith
Copy link
Collaborator

Ralith commented Feb 3, 2019

Also I just realized that if we split ash into ash-sys then we can't easily extend StructureType like this any more.

Don't extensions need code in ash-sys anyway, for function signatures and struct types? Doesn't seem like this is any different to me.

@MaikKlein
Copy link
Member

Don't extensions need code in ash-sys anyway, for function signatures and struct types? Doesn't seem like this is any different to me.

Yes it is not a real problem, we just have to move the "low level" part into ash-sys.

@msiglreith msiglreith changed the title [WIP] Experimental AMD extensions Experimental AMD extensions Feb 24, 2019
@msiglreith
Copy link
Contributor Author

Should be ready now. I skipped the high level API as I'm not sure if it would be of much worth for now

Copy link
Member

@MaikKlein MaikKlein left a comment

Choose a reason for hiding this comment

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

Looks good to me. Leaving out the high level wrapper is fine. Have you already played around with it?

@msiglreith
Copy link
Contributor Author

A bit, mostly trying to make sense out of the exposed counters as 90% of these are not documented and GCN generation specific 😅
Querying the device capabilities works fine so far at least. I need to look deeper inside GPA.

@MaikKlein
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Feb 25, 2019
184: Experimental AMD extensions r=MaikKlein a=msiglreith

Addresses #178 (comment)

Not fully supports all parts of the APIs, just trying to get some general feedback if this meets the expected structuring.

Co-authored-by: msiglreith <[email protected]>
@bors
Copy link
Contributor

bors bot commented Feb 25, 2019

@bors bors bot merged commit 2f60510 into ash-rs:master Feb 25, 2019
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