Skip to content

Conversation

@bjjones
Copy link
Contributor

@bjjones bjjones commented Oct 12, 2022

Removes usage of a callback using std::function in favor of a C-style function pointer callback.

@bjjones
Copy link
Contributor Author

bjjones commented Oct 12, 2022

This hasn't been tested. What is the best way to test the MVI? Building the WIP DML Backend patch right now to try testing with the full thing.

I'm a little unsure if I got the ComPtr operations correct - it's been awhile.

@bjjones bjjones requested a review from bbernhar October 12, 2022 18:10
@bbernhar
Copy link
Contributor

This hasn't been tested. What is the best way to test the MVI?

Probably need to come up with a end2end test specifically for the MVI (really basic), and perhaps re-use it for the Chromium CL.

@bbernhar
Copy link
Contributor

Approach LGTM + left some comments.

@github-actions github-actions bot added Build Changes to build system. D3D12 DirectX 12 Backend Change labels Oct 17, 2022
@github-actions github-actions bot added Test Changes in tests. and removed Build Changes to build system. labels Oct 18, 2022
@bbernhar
Copy link
Contributor

Dawn integration patch will also need to be updated too. Just apply the https://github.com/intel/GPGMM/blob/main/.github/workflows/.patches/dawn.diff to a local (Dawn) repo, modify it, then save the new diff. I keep the Dawn bot going for API coverage.

@bjjones
Copy link
Contributor Author

bjjones commented Oct 19, 2022

Is the dawn.patch supposed to apply cleanly to top-of-tree Dawn? I'm getting multiple conflicts and even more errors when I try to build.

@bbernhar
Copy link
Contributor

Is the dawn.patch supposed to apply cleanly to top-of-tree Dawn? I'm getting multiple conflicts and even more errors when I try to build.

Possibly but it's currently based on https://github.com/intel/GPGMM/blob/main/scripts/dawn.deps (be4c9f48aaef9f2144a654a847b76a6a0050ec5b).

Copy link
Contributor

@bbernhar bbernhar left a comment

Choose a reason for hiding this comment

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

LGTM + after fixes.

@bjjones
Copy link
Contributor Author

bjjones commented Oct 20, 2022

I'm getting hung up on updating the dawn.diff. It applies cleanly on the version you've stated, but I then I get a number of build errors. I also need to be using near top of tree GPGMM if I want to apply this CreateHeap change - and the build errors only get worse after that.

@bbernhar
Copy link
Contributor

I'm getting hung up on updating the dawn.diff. It applies cleanly on the version you've stated, but I then I get a number of build errors. I also need to be using near top of tree GPGMM if I want to apply this CreateHeap change - and the build errors only get worse after that.

Normally I just edit + commit diff and let the builder do its thing. Otherwise, the steps to build locally are here: https://github.com/intel/GPGMM/blob/main/.github/workflows/win_dawn_rel.yaml. Typically, a git checkout followed by gclient sync is all I need.

@bjjones bjjones force-pushed the fixCallback2 branch 4 times, most recently from a5031e9 to 78c0f5f Compare October 24, 2022 17:20
@bjjones bjjones force-pushed the fixCallback2 branch 2 times, most recently from d234bda to 62e50e6 Compare October 24, 2022 19:18
@bjjones bjjones merged commit 872b882 into intel:main Oct 24, 2022
@bbernhar bbernhar added the breaking-change Breaking Change label Oct 24, 2022
@bbernhar
Copy link
Contributor

@bjjones Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Breaking Change D3D12 DirectX 12 Backend Change Test Changes in tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants