Skip to content
This repository was archived by the owner on Apr 23, 2021. It is now read-only.

Conversation

@whchung
Copy link

@whchung whchung commented Jul 31, 2019

  • Introduce GPUToROCDL to map thread/block/grid device functions to corresponding intrinsics and device functions on AMD GPU.
  • Introduce GPUToROCm to drive MLIR->LLVM->HSA code object lowering process. Some passes are pretty similar with those in GPUToCUDA and may deserve to be merged.
  • Introduce mlir-rocm-runner in mlir/tools.

Authors:

@googlebot
Copy link
Collaborator

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@whchung whchung force-pushed the exp-mlir-rocm-runner branch from 4b7e965 to d53b8d9 Compare July 31, 2019 16:25
@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@joker-eph
Copy link
Contributor

Hi,

Thanks for the contribution! It is nice to see another GPU target :)

What is the status of this pull-request right now? Are you seeking early feedback or are you looking for a quick path to integrate this upstream?
I saw with a quick scan things like git clone -b exp-mlir-rocm-runner https://github.com/whchung/mlir added to the README which lead me to be unsure about the current state.

@whchung
Copy link
Author

whchung commented Jul 31, 2019

@joker-eph Thanks for your prompt reply.

All tests are passing on this PR, which means we can run MLIR core end-to-end on AMD ROCm platform.

$ cmake --build . --target check-mlir         
[0/1] Running the MLIR regression tests
Testing Time: 5.13s
  Expected Passes    : 223
  Unsupported Tests  : 4

4 unsupported tests are those on CUDA platform.

We are starting building internal tools based on work derived from this PR, so we'd like to understand the steps to get the PR revised and merged.

Granted there are quite a few duplication between GPUToCUDA and GPUToROCm at this moment, plus there are some hard-coded logic at this moment so I think some discussion would be surely needed.

I'll remove my change to README.md. They should stay downstream.

@whchung whchung force-pushed the exp-mlir-rocm-runner branch 2 times, most recently from 338e28e to 7534424 Compare July 31, 2019 17:54
@joker-eph
Copy link
Contributor

All tests are passing on this PR, which means we can run MLIR core end-to-end on AMD ROCm platform.

Fantastic!

Granted there are quite a few duplication between GPUToCUDA and GPUToROCm at this moment, plus there are some hard-coded logic at this moment so I think some discussion would be surely needed.

Right, I suspect we will want to integrate this piece by piece, in PRs as small as possible.
We will also look into avoiding as much duplication as possible and instead refactor and reuse the logic in the repo.

I believe it would be best to start with an RFC on the public mailing-list to discuss this and involve the folks actively working on GPU targets already (SPIR-V and Cuda).

Thanks!

@whchung
Copy link
Author

whchung commented Aug 1, 2019

@joker-eph Thanks. I'll revise this PR and break it into smaller patches, probably tablegen for ROCDL dialect to begin with. Also I'll send an RFC to the mailing list.

@whchung whchung force-pushed the exp-mlir-rocm-runner branch from 76dc57f to 0ca81f7 Compare August 2, 2019 16:08
@whchung whchung changed the title [ROCm] Experimental logic to enable MLIR on AMD ROCm platform. [ROCm] CMake and lit changes to enable AMDGPU support. Aug 2, 2019
@whchung
Copy link
Author

whchung commented Aug 2, 2019

@joker-eph I've reduced this PR to a minimal size. Will file subsequent ones moving forward, plus submitting RFCs to the public mailing list.

@whchung whchung mentioned this pull request Aug 2, 2019
@joker-eph
Copy link
Contributor

Thanks!

From what I see in the diff now, these are mostly only build configuration changes left? I expect these to land with the actual runner in one of the later PRs.

@whchung
Copy link
Author

whchung commented Aug 2, 2019

@joker-eph Yeah, come to think of it there is really no meat in this PR now.

Let me keep this PR open for awhile, and use this PR as a placeholder for mlir-rocm-runner then.

@joker-eph
Copy link
Contributor

Closing for now (cleanup), feel free to re-open as needed though!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants